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

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Other than that ... Ship it ! I'm fine with the change and happy to maintain it in the future.

INLINE COMMENTS

> rkflx wrote in breezedecoration.cpp:643
> 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).

Matter of taste. 
Some people (including me), like to have the default (fallback) choice first. 
Some others like to have it last.
Some, inline.
There is no c++ rule about this as far as I know.

> rkflx wrote in breezeconfigwidget.cpp:91
> Both should be `ShadowVeryLarge`, otherwise when reopening the config (as well as restarting KWin) `Large` would be shown despite setting a bigger shadow beforehand.

No
Only the first should be ShadowVeryLarge
The second (l92) should remain "ShadowLarge", or rather whatever the default shadow size we want. 
The logic behind this is that if the shadowSize is "invalid" (meaning: larger than the largest possible value), you fallback to the default value. 
This way we gracefully reset all previously set sizes (from spinbox) to the default size.

> rkflx wrote in breezeshadowhelper.cpp:57
> Please order by size here, too.

idem

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/e6b050ef/attachment-0001.html>