[Differential] [Commented On] D3706: WIP: Global Menu Applet

[davidedmundson (David Edmundson)]

davidedmundson added inline comments.

INLINE COMMENTS

> appmenuapplet.cpp:141
> +        if (e->key() == Qt::Key_Left) {
> +            emit requestActivateIndex(m_currentIndex - 1);
> +            return true;

It's more normal to do the bounding to the right value before you emit a signal, rather than in the signal handler in QML

Otherwise your passing bad data around, which is the sort of thing that breaks later.

> appmenuapplet.cpp:159
> +
> +        // FIXME the panel margin breaks Fitt's law :(
> +        const QPointF &localPos = m_buttonGrid->mapFromGlobal(e->globalPos());

it shouldn't do, the panel will take the event, and re-emit a new one in the applet position

> broulik wrote in main.qml:25
> Can you perhaps look into making it a c++ applet rather than using a private import – we somewhat try to get away from this

> we somewhat try to get away from this

Why?

> appmenumodel.cpp:85
> +        if (actions.isEmpty()) {
> +            return;
> +        }

If you return here you have an unmatching begin/remove

> appmenumodel.cpp:108
> +
> +            const xcb_intern_atom_cookie_t atomCookie = xcb_intern_atom(c, false, name.length(), name.constData());
> +            QScopedPointer<xcb_intern_atom_reply_t, QScopedPointerPodDeleter> atomReply(xcb_intern_atom_reply(c, atomCookie, Q_NULLPTR));

atoms can be cached in the ctor

REPOSITORY
  R120 Plasma Workspace

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

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: chinmoyr, #plasma, broulik
Cc: davidedmundson, plasma-devel, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20161218/e2e260d5/attachment.html>