D9627: Use combobox to choose shadow size and use more appropriate values for menu & tooltip shadow sizes

Henrik Fehlauer noreply at phabricator.kde.org
Sat Jan 6 20:41:30 UTC 2018


rkflx added a comment.


  In https://phabricator.kde.org/D9627#186696, @rkflx wrote:
  
  > I won't need to test again, as the screenshots clearly show it working fine ;)
  
  
  Never trust a screenshot…

INLINE COMMENTS

> breezedecoration.cpp:643
> +            {
> +                default:
> +                case InternalSettings::ShadowLarge: shadowSize = 64; break;

Ordering by size would be more readable, `default` doesn't have to sit at the top (it should just tag the currently favoured default option).

> breezeconfigwidget.cpp:91
>          // load shadows
> -        m_ui.shadowSize->setValue( m_internalSettings->shadowSize() );
> +        if( m_internalSettings->shadowSize() <= InternalSettings::ShadowLarge ) m_ui.shadowSize->setCurrentIndex( m_internalSettings->shadowSize() );
> +        else m_ui.shadowSize->setCurrentIndex( InternalSettings::ShadowLarge );

Both should be `ShadowVeryLarge`, otherwise when reopening the config (as well as restarting KWin) `Large` would be shown despite setting a bigger shadow beforehand.

> breezeshadowhelper.cpp:57
> +        {
> +            default:
> +            case Breeze::StyleConfigData::ShadowLarge: return 16;

Please order by size here, too.

REPOSITORY
  R31 Breeze

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D9627

To: ngraham, #vdg, #breeze, hpereiradacosta, abetts
Cc: rkflx, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20180106/29a596d3/attachment.html>


More information about the Plasma-devel mailing list