[Differential] [Commented On] D3089: Restore global menu support

[graesslin (Martin Gräßlin)]

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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161102/c2cac2c8/attachment-0001.html>