D10480: align checkable widgets in menu items

Hugo Pereira Da Costa noreply at phabricator.kde.org
Tue Feb 13 13:24:57 UTC 2018


hpereiradacosta added a comment.


  the diff appears more complex than it actually is because of unrelated changes. Please keep the changes to the minimum, this will help reviewing.

INLINE COMMENTS

> breeze.h:70
>          Menu_FrameWidth = 0,
> -        MenuItem_MarginWidth = 3,
> +        MenuItem_MarginWidth = 4,
>          MenuItem_ItemSpacing = 4,

This change is unrelated with the centering. Should be another patch.

> breezestyle.cpp:4660
>  
> +            return true;
>          }

this change is unrelated. Please revert.

> breezestyle.cpp:4701
> +
> +        QRect arrowRect(
> +            contentsRect.right() - Metrics::MenuButton_IndicatorWidth + 1,

why was this chunk of code moved ? This is unrelated to the change.
Please try keep the diff to the minimum

> breezestyle.cpp:4744
>  
> -            checkBoxRect = visualRect( option, checkBoxRect );
> +            checkableRect = visualRect( option, checkableRect );
>  

please dont rename variables just for the safe of it, and keep checkboxrect.

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: 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/20180213/741087a3/attachment.html>


More information about the Plasma-devel mailing list