D5111: Provide demo/preview for checkable menu items

[Anthony Fieroni]

anthonyfieroni removed a reviewer: anthonyfieroni.
Herald added a project: Plasma.

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, hpereiradacosta, jriddell, zhigalin
Cc: ltoscano, kde-mac, #frameworks, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D5111: Provide demo/preview for checkable menu items

[Anthony Fieroni]

anthonyfieroni added a dependent revision: D8741: [kget] Fix a crash when 
opening the transfer history dialog.

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, hpereiradacosta, jriddell, zhigalin, anthonyfieroni
Cc: ltoscano, kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items

[Anthony Fieroni]

anthonyfieroni removed a dependent revision: D8741: [kget] Fix a crash when 
opening the transfer history dialog.

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, hpereiradacosta, jriddell, zhigalin, anthonyfieroni
Cc: ltoscano, kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items

[René J.V. Bertin]

rjvbb retitled this revision from "Provide demo/preview for checkable menu 
items and colour scheme comparison" to "Provide demo/preview for checkable menu 
items".
rjvbb edited the summary of this revision.
rjvbb edited the test plan for this revision.
rjvbb set the repository for this revision to R113 Oxygen Theme.

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, hpereiradacosta, jriddell, zhigalin, anthonyfieroni
Cc: ltoscano, kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J.V. Bertin]

rjvbb updated this revision to Diff 15812.
rjvbb added a comment.


  Updated for 5.10.2+

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5111?vs=12788=15812

REVISION DETAIL
  https://phabricator.kde.org/D5111

AFFECTED FILES
  kstyle/demo/main.cpp
  kstyle/demo/oxygendemodialog.cpp
  kstyle/demo/oxygendemodialog.h
  kstyle/demo/oxygenmdidemowidget.cpp

To: rjvbb, hpereiradacosta, jriddell, zhigalin, anthonyfieroni
Cc: ltoscano, kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb updated this revision to Diff 12788.
rjvbb added a comment.


  Rebased on master; also updated the MDI page title to indicate that it has 
menu previews.
  
  In terms of general improvement we might consider adding a command line 
argument to start with a specific page.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5111?vs=12666=12788

REVISION DETAIL
  https://phabricator.kde.org/D5111

AFFECTED FILES
  kstyle/demo/main.cpp
  kstyle/demo/oxygendemodialog.cpp
  kstyle/demo/oxygendemodialog.h
  kstyle/demo/oxygenmdidemowidget.cpp

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: ltoscano, kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb added a comment.


  In https://phabricator.kde.org/D5111#96517, @ltoscano wrote:
  
  > Which repository is this? Oxygen? Please add it.
  
  
  Apologies, something must have gone wrong adding it the 1st time.

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: ltoscano, kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb set the repository for this revision to R113 Oxygen Theme.

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: ltoscano, kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[Luigi Toscano]

ltoscano added a comment.


  Which repository is this? Oxygen? Please add it.

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: ltoscano, kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb updated this revision to Diff 12666.
rjvbb added a comment.


  Maintain Qt4 compatibility

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5111?vs=12647=12666

REVISION DETAIL
  https://phabricator.kde.org/D5111

AFFECTED FILES
  kstyle/demo/main.cpp
  kstyle/demo/oxygendemodialog.cpp
  kstyle/demo/oxygendemodialog.h
  kstyle/demo/oxygenmdidemowidget.cpp

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb updated this revision to Diff 12647.
rjvbb added a comment.


  stripped diff.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5111?vs=12644=12647

REVISION DETAIL
  https://phabricator.kde.org/D5111

AFFECTED FILES
  kstyle/demo/oxygendemodialog.cpp
  kstyle/demo/oxygendemodialog.h
  kstyle/demo/oxygenmdidemowidget.cpp

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb added a comment.


  Colour scheme chooser proposition split off: https://phabricator.kde.org/D5113

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb updated this revision to Diff 12644.
rjvbb edited the summary of this revision.
rjvbb added a comment.


  Updated, not yet split.
  
  I've kept the former left-to-right menu action as a "check here" noop action 
and made the distinction a bit more explicit by adding a menu section (which 
also didn't didn't benefit from a preview yet).
  
  FWIW: notice how Qt flips "<- Check here" to "Check here ->" automatically 
when flipping the layout! Should a comment about this feature be added so that 
translators will preserve it?

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D5111?vs=12634=12644

REVISION DETAIL
  https://phabricator.kde.org/D5111

AFFECTED FILES
  kstyle/demo/CMakeLists.txt
  kstyle/demo/oxygendemodialog.cpp
  kstyle/demo/oxygendemodialog.h
  kstyle/demo/oxygenmdidemowidget.cpp
  kstyle/demo/oxygenschemechooser.cpp
  kstyle/demo/oxygenschemechooser.h

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb added a comment.


  Once more. Note how Oxygen renders an icon on the menu button despite me 
having disabled the icons-in-buttons feature in the settings. Only QtCurve 
seems to respect this setting for regular buttons (in dialog button boxes) 
nowadays.
  
  F2988220: oxydemo-oxygen-menu.png 

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb added a comment.


  Screenshots:
  
  F2988075: oxydemo-breeze.png 
  
  F2988076: oxydemo-qtcurve.png 
  
  F2988077: oxydemo-oxygen.png 
  
  F2988078: oxydemo-qtcurve-menu.png 

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

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

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Hi,
  Thanks for the set of patches.
  
  in general, i am ok with the change but:
  
  - 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 ...)
  - right to left layout action <- no. There is already one at the bottom of 
the window.
  - 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 ?
  - 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. So the whole corresponding code should go, right ? Or is 
it work in progress ? Personally I would disagree with having oxygen-demo being 
anything other than a demo, and for instance altering configuration. This is 
not the right place. The right place is the relevant KCM dialog.
  
  Best,
  
  Hugo

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, jriddell, anthonyfieroni, zhigalin, hpereiradacosta
Cc: kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb removed a project: Plasma.
rjvbb removed a subscriber: plasma-devel.

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

To: rjvbb, hpereiradacosta, jriddell, anthonyfieroni, zhigalin
Cc: kde-mac, #frameworks

D5111: Provide demo/preview for checkable menu items and colour scheme comparison

[René J . V . Bertin]

rjvbb created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.

REVISION SUMMARY
  This is a continuation/transfer of https://git.reviewboard.kde.org/r/128109/ 
(which still has a few screenshots)
  
  There currently is no "official" cheap way to preview how checked menu items 
are rendered as a function of installed widget style, nor an easy/cheap way to 
compare the look of a given widget style under different colour schemes. I have 
been annoyed by the fact one has to (re)launch often heavy applications to 
assess the impact of changes to these look-and-feel aspects.
  
  This patch explores a straightforward way of adding that missing 
functionality via the most exhaustive look-and-feel preview utility I am aware 
of that is cheap in resource requirements: Oxygen's demo app.
  
  This demo already has menus in the MDI preview. The patch makes the layout 
items checkable and puts them in a QActionGroup so their mutually exclusive 
nature is taken into account. It also adds a "right to left" non-exclusive 
checkable item which is linked to the "Left to Right" checkbox in the demo 
frame.
  
  For colour scheme exploration I adapted a thin wrapper around 
KColorSchemeManager written originally by Alexander Zhigalin for KDevelop; it 
is used to add a button with drop-down menu to the page widget's button box 
which makes it available in all views.

TEST PLAN
  The menu additions have been checked on Linux and Mac OS X with Qt 5.6.0 - 
5.8.0 and frameworks 5.22.0 - 5.32.0; the colour scheme addition with Qt 5.8.0 
and frameworks 5.32.0 .
  
  I looked for an appropriate way to uncheck the Tile/Cascade/Tabbed menu items 
(= when the user moves or resizes one of the MDI windows) but it seems those 
events are not available through signals and would thus require subclassing 
MdiSubWindow .
  
  It would make sense IMHO to separate this demo from Oxygen. It has nothing 
Oxygen-specific and could be provided as a standalone theme explorer or bundled 
with another package that doesn't have too strong connotations of being 
"Plasma-exclusive" (Qt has widget style and palette support on most if not all 
platforms it runs on).

REPOSITORY
  R113 Oxygen Theme

REVISION DETAIL
  https://phabricator.kde.org/D5111

AFFECTED FILES
  kstyle/demo/CMakeLists.txt
  kstyle/demo/oxygendemodialog.cpp
  kstyle/demo/oxygendemodialog.h
  kstyle/demo/oxygenmdidemowidget.cpp
  kstyle/demo/oxygenmdidemowidget.h
  kstyle/demo/oxygenschemechooser.cpp
  kstyle/demo/oxygenschemechooser.h

To: rjvbb, hpereiradacosta, jriddell, anthonyfieroni, zhigalin
Cc: kde-mac, plasma-devel, #frameworks, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol