[KDE/Mac] Review Request 120149: [OS X] improved "system tray" menus

[René J.V. Bertin]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120149/
-----------------------------------------------------------

Review request for KDE Software on Mac OS X, Marco Martin and Olivier Goffart.


Repository: kdelibs


Description
-------

Until now, "system tray" menus had some rendering issues on Mac OS X:

- The menu title, the 1st menu item that on Linux shows the application name, remained empty
- Menu items that can (in principle, potentially) show an icon always showed the icon

Point 1 was resolved by emulating the Linux addTitle/setTitle action in `KStatusNotifierItemPrivate::init()` : the menu title is implemented as a deactive standard menuitem followed by a separator. This makes the item stand out on a GUI that doesn't support the kind of formatting in menus as used in the Linux implementation.

Point 2 was identified as a Qt issue: `isIconVisibleInMenu` is ignored for systray menus. It was resolved by adding `KMenu::addAction` methods that overload the ones from QMenu that were hitherto inherited unchanged by KMenu. The only different method is `addAction(QAction*)` which removes the icon from the `QAction` if `isIconVisibleInMenu()` is false. The other `addAction` methods are "overloaded with themselves" with `using QMenu::addAction;` in the header file.


Diffs
-----

  kdeui/notifications/kstatusnotifieritem.cpp 1b15d40 
  kdeui/widgets/kmenu.h f96e263 
  kdeui/widgets/kmenu.cpp 7dab149 

Diff: https://git.reviewboard.kde.org/r/120149/diff/


Testing
-------

Testing was done with kdelibs git/master and KDE/MacPorts on OS X 10.6.8 . The modified code is in compile-time conditional blocks used only on OS X, so no regressions are to be expected on other platforms.

KF5 is not production ready on OS X, so I am not currently able to port these modifications beyond KDE4. However, I did see that Qt5 has a new approach to adding titles to menus, which can be described as a "labelled separator". Backporting that function from the Qt5 source to kdelibs gave menu items that had the separator but not the text (title) label. It is thus likely that some kind of emulation will also be required with KF5, on OS X.

I considered doing the addTitle/setTitle emulation in kmenu.cpp, but decided against that for now. Menu titles are rendered as under Linux in menus that are not attached to the OS X toplevel menubar (say in context menus). Without knowing how to distinguish the kind of menu in KMenu methods the emulation will have to remain in the client code.


Thanks,

René J.V. Bertin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-mac/attachments/20140911/405cf6ea/attachment.html>