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

Reply via email to