D5111: Provide demo/preview for checkable menu items and colour scheme comparison
René J.V. Bertin
noreply at phabricator.kde.org
Mon Mar 20 12:19:28 UTC 2017
rjvbb added a comment.
> - please re-add the screenshot from Review Board. (sorry I was not aware of this review request cause I was not in the list of reviewers, even though official maintainer of oxygen ...)
Sorry about that, I thought you'd be a member of the Plasma group. But I had a hunch so added a few reviewers manually this time.
> - right to left layout action <- no. There is already one at the bottom of the window.
The goal was not to duplicate functionality, but to add an additional item to the Layout menu that was not mutually exclusive with the others, as different styles can render such items differently. It also gives the possibility to add a separator.
IIRC the patch *would* become somewhat simpler if I just leave the menu item but make it a noop. Would you be OK with that?
> - this review should really be several, one per feature: one for the checkboxes/radiobuttons in the mdi window, one for the colorschemechooser. Can you split ?
Yay, extra work... Fortunately not too much, will do later today.
> - finally, there is need for more detail review (once above is done). For instance, in ColorSchemeChooser you use SUPPORT_THEME_SAVING, but this one is set/defined nowhere.
TBH, I was more or less hoping for feedback like this. I also don't see the need to save this setting but didn't want to strip it out immediately, also out of some form of respect for Alexander's original work.
REPOSITORY
R113 Oxygen Theme
REVISION DETAIL
https://phabricator.kde.org/D5111
To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: kde-mac, #frameworks
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20170320/27285f5f/attachment.html>
More information about the kde-mac
mailing list