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
