[Differential] [Accepted] D4620: allow to add application actions on an open menu

David Faure noreply at phabricator.kde.org
Sat Feb 18 10:35:21 UTC 2017


dfaure accepted this revision.
dfaure added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> dropjob.cpp:330
> +        }
> +        //separator at last position and not needed anymore?
> +        if (d->m_appActions.isEmpty() && d->m_pluginActions.isEmpty() &&

Don't worry too much about that, QMenu automatically suppresses doubled separators or separators at begin/end of the menu.
So I bet this if block isn't necessary in practice.

> dropjob.cpp:332
> +        if (d->m_appActions.isEmpty() && d->m_pluginActions.isEmpty() &&
> +            menu->actions().last()->isSeparator()) {
> +            menu->removeAction(menu->actions().last());

If you do keep it, maybe add a check that menu->actions() isn't empty, otherwise last() will assert.

REPOSITORY
  R241 KIO

BRANCH
  phab/setActionsLater

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

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

To: mart, #plasma, dfaure
Cc: dfaure, hein, plasma-devel, #frameworks, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20170218/583ee32e/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list