graesslin added a comment.
One more general thing: I'd love to see a test case for the new added code.
INLINE COMMENTS
> abstract_client.cpp:75-77
> + connect(ApplicationMenu::self(),
> &ApplicationMenu::applicationMenuEnabledChanged, this, [this] {
> + emit hasApplicationMenuChanged(hasApplicationMenu());
> + });
why carry the bool in the signal? That seems to make it way more complicated
here
> abstract_client.cpp:1715
> + decoration()->showApplicationMenu();
> + // we don't know where the application menu button will be, show it in
> the top left corner instead
> + } else {
that comment should be together with the else, shouldn't it?
> abstract_client.h:1043
> static bool s_haveResizeEffect;
> +
> };
added empty new line
> appmenu.cpp:102-107
> +{
> + return Workspace::self()->findAbstractClient([&](const AbstractClient
> *c) {
> + return c->applicationMenuServiceName() == serviceName
> + && c->applicationMenuObjectPath() == menuObjectPath.path();
> + });
> +}
add a safety check in case the serviceName and/or menuObjectPath is empty and
return null?
REPOSITORY
rKWIN KWin
REVISION DETAIL
https://phabricator.kde.org/D3089
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: broulik, #plasma
Cc: graesslin, plasma-devel, kwin, lesliezhai, ali-mohamed, hardening,
jensreuterberg, abetts, sebas
