--===============0594243008765988974== Content-Type: multipart/alternative; boundary="===============1578884550129332118==" --===============1578884550129332118== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit > On 六月 4, 2016, 4:02 p.m., Yichao Yu wrote: > > LGTM, this should be added to the configure UI. The tab for popup menu should work. The .ui file needs to be updated to include the checkbox for this and a corresponding rule should be added to the .cpp file (There should already be a number of boolean options in that tab that you can copy.) > > René J.V. Bertin wrote: > I can have another look, thinking it should be easy to figure out where and how the "Menu items" UI options were set up, but got lost. I see that I should have opened the qtcurveconfigbase.ui file in a text editor. That at least clears up where the controls are defined, but there's still a lot of things going on in the code that undoubtedly made it easy for the original author but not to someone jumping in for a casual change! Just to be clear, I've had a look at that file (possibly with small tweaks) but I'm not the original author either.... The UI file should be fairly standard and either kdevelop or qtcreator/qtdesigner should be able to edit it (text editor should work too). This is also why I suggested copying a nearby option. `grep`ing the widget id worked pretty well last time I did something simlar. - Yichao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128097/#review96189 ----------------------------------------------------------- On 六月 4, 2016, 3:53 p.m., René J.V. Bertin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128097/ > ----------------------------------------------------------- > > (Updated 六月 4, 2016, 3:53 p.m.) > > > Review request for KDE Software on Mac OS X, Plasma and Yichao Yu. > > > Repository: qtcurve > > > Description > ------- > > This introduces an option (hidden for now) to adorn checked menu items with only a check mark rather than the same widget that is used for checkboxes or radio buttons (for sets of mutually exclusive menu items). > > Initially I implemented this by simply skipping the widget "box" and drawing only the checkbox tick for both kinds of menu items (cf. https://bugs.kde.org/show_bug.cgi?id=363895). I then realised that this looks weird when the user uses a very tall or tiny font (or has a high DPI screen). Therefore the check is now generated using the UniCode `Check Mark` glyph (? cf. http://www.fileformat.info/info/unicode/char/2713/index.htm) rendered in the menu font (or `Apple Symbols`, on OS X). > > A new member is introduced in the `Options` structure that controls this new behaviour. Its value is read from and written to the config file, but I have not yet implemented its UI control through the configuration interface. I'll want some guidance for that step. > > I propose to make this the default behaviour on OS X, so that popup menus can be closer in appearance to the native menus from the toplevel menubar (those menus are not rendered through Qt, and use a single check mark too). > > The GTk2 style already rendered checked menu items like this so I did not change anything there (and don't really plan to touch that code at all). > > > Diffs > ----- > > gtk2/common/common.h cb0ec87 > gtk2/common/config_file.cpp 96936e2 > qt4/common/common.h 313db33 > qt4/common/config_file.cpp c58ad1a > qt4/style/qtcurve.cpp 951ec1a > qt5/common/common.h bb103fd > qt5/common/config_file.cpp 362381a > qt5/style/qtcurve_api.cpp b8535da > qt5/style/qtcurve_primitive.cpp a8a2bed > > Diff: https://git.reviewboard.kde.org/r/128097/diff/ > > > Testing > ------- > > On OS X 10.9 and Linux with Qt 4.8.7 and Qt 5.6.0 . > > > Thanks, > > René J.V. Bertin > > --===============1578884550129332118== MIME-Version: 1.0 Content-Type: text/html; charset="utf-8" Content-Transfer-Encoding: 8bit
This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128097/

On 六月 4th, 2016, 4:02 p.m. EDT, Yichao Yu wrote:

LGTM, this should be added to the configure UI. The tab for popup menu should work. The .ui file needs to be updated to include the checkbox for this and a corresponding rule should be added to the .cpp file (There should already be a number of boolean options in that tab that you can copy.)

On 六月 4th, 2016, 4:24 p.m. EDT, René J.V. Bertin wrote:

I can have another look, thinking it should be easy to figure out where and how the "Menu items" UI options were set up, but got lost. I see that I should have opened the qtcurveconfigbase.ui file in a text editor. That at least clears up where the controls are defined, but there's still a lot of things going on in the code that undoubtedly made it easy for the original author but not to someone jumping in for a casual change!
Just to be clear, I've had a look at that file (possibly with small tweaks) but I'm not the original author either.... The UI file should be fairly standard and either kdevelop or qtcreator/qtdesigner should be able to edit it (text editor should work too).


This is also why I suggested copying a nearby option. greping the widget id worked pretty well last time I did something simlar.

- Yichao


On 六月 4th, 2016, 3:53 p.m. EDT, René J.V. Bertin wrote:

Review request for KDE Software on Mac OS X, Plasma and Yichao Yu.
By René J.V. Bertin.

Updated 六月 4, 2016, 3:53 p.m.

Repository: qtcurve

Description 

This introduces an option (hidden for now) to adorn checked menu items with only a check mark rather than the same widget that is used for checkboxes or radio buttons (for sets of mutually exclusive menu items).


Initially I implemented this by simply skipping the widget "box" and drawing only the checkbox tick for both kinds of menu items (cf. https://bugs.kde.org/show_bug.cgi?id=363895). I then realised that this looks weird when the user uses a very tall or tiny font (or has a high DPI screen). Therefore the check is now generated using the UniCode Check Mark glyph (? cf. http://www.fileformat.info/info/unicode/char/2713/index.htm) rendered in the menu font (or Apple Symbols, on OS X).


A new member is introduced in the Options structure that controls this new behaviour. Its value is read from and written to the config file, but I have not yet implemented its UI control through the configuration interface. I'll want some guidance for that step.


I propose to make this the default behaviour on OS X, so that popup menus can be closer in appearance to the native menus from the toplevel menubar (those menus are not rendered through Qt, and use a single check mark too).


The GTk2 style already rendered checked menu items like this so I did not change anything there (and don't really plan to touch that code at all).

Testing 

On OS X 10.9 and Linux with Qt 4.8.7 and Qt 5.6.0 .

Diffs

  • gtk2/common/common.h (cb0ec87)
  • gtk2/common/config_file.cpp (96936e2)
  • qt4/common/common.h (313db33)
  • qt4/common/config_file.cpp (c58ad1a)
  • qt4/style/qtcurve.cpp (951ec1a)
  • qt5/common/common.h (bb103fd)
  • qt5/common/config_file.cpp (362381a)
  • qt5/style/qtcurve_api.cpp (b8535da)
  • qt5/style/qtcurve_primitive.cpp (a8a2bed)

View Diff
--===============1578884550129332118==-- --===============0594243008765988974== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KUGxhc21hLWRl dmVsIG1haWxpbmcgbGlzdApQbGFzbWEtZGV2ZWxAa2RlLm9yZwpodHRwczovL21haWwua2RlLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL3BsYXNtYS1kZXZlbAo= --===============0594243008765988974==--