D10480: align checkable widgets in menu items

Hugo Pereira Da Costa noreply at phabricator.kde.org
Tue Feb 13 19:51:18 UTC 2018


hpereiradacosta added a comment.


  Ok. So, the patch works, but ... there is something fishy here. In principle it should not be needed. Right ? 
  Ifcheckboxes have a fixed size, and if margins would be equal to spacing, and if the math are correct, having something laied out as: margin + checkbox + spacing + icon + spacing + text would be automatically centered, no ? 
  Idem without icon. (and if there is double spacing, then it is a bug).
  At least this is how the code is supposed to work. If not, it should be fixed, rather than circomvoluting the issue with some centering. 
  (note that the padding on the icons is disconnected to this: not all icons have padding, especially if you change icon theme, so you cannot do anything but rely on the icon size.
  
  Bottom line, I would like to investigate the current code (which _should_ work), rather than accepting this patch. 
  Or do I miss something ?
  
  The reason why I would rather fix the current code than introducing some centering, is because centering would automatically hide deeper problems, and because it makes the metrics (margin, icon width, spacing), not reflect what the code do. Which then introduces confusion and make the code not maintainable.

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/8cdcc88c/attachment.html>


More information about the Plasma-devel mailing list