D10480: align checkable widgets in menu items

Hugo Pereira Da Costa noreply at phabricator.kde.org
Fri Feb 16 10:02:14 UTC 2018


hpereiradacosta added a comment.


  In D10480#207511 <https://phabricator.kde.org/D10480#207511>, @zzag wrote:
  
  > > Here: if you look at the last checkbox (folders first). The distance to the left edge is larger to that of the bottom edge, while before they were identical.
  >
  > Oh, I haven't noticed that before. To me, that's still fine.
  >
  > > So this really depends on what we want to align with what ...
  >
  > I'd propose to align check boxes between the left edge and icon/text.
  
  
  ok. I agree.
  
  See comments below: if you can split this patch in two (first bug fix, then margin width increase, then both can go.
  
  Thanks for the good work !

INLINE COMMENTS

> breezestyle.cpp:4740
> +        QRect iconRect;
> +        if( showIcon && iconWidth > 0 )
> +        {

If I understand right, this is the double spacing bug fix. 
Correct ? Very nice. 
In principle, it would be better to have it in a separate Review, and a separate commit. 
This way, if in the future someone wants to revert the margin change, she/he does not revert the bug fix at the same time. Can you do that ? 
Note that thinking about it, I also like the other fix (the margin width increase), so this will go too.

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/20180216/5b78e5b5/attachment.html>


More information about the Plasma-devel mailing list