[Differential] [Updated] D4739: make sure the cancel action is last

[David Faure]

dfaure added a comment.


  I like the encapsulation into a different class.

INLINE COMMENTS

> dropjob.cpp:63
> +public:
> +    DropMenu(QWidget *parent = 0);
> +    ~DropMenu();

nullptr

> dropjob.cpp:167
> +
> +void DropMenu::addExtraActions(QList<QAction *> appActions, QList<QAction *> pluginActions)
> +{

unnecessary copying (refcounting), the two QLists should be passed as const ref.

> dropjob.cpp:173
> +    removeAction(m_extraActionsSeparator);
> +    for (QAction *action : m_appActions) {
> +        removeAction(action);

(which would avoid the detaching here)

> dropjob.cpp:350
>      popup->addSeparator();
> -    popup->addAction(popupCancelAction);
> +//    popup->addAction(popupCancelAction);
>  }

please clean up

> dropjob.h:38
>  class DropJobPrivate;
> +class DropMenu;
>  

I don't think this is needed, nothing else in the header refers to it (I guess it was an initial solution with private slots that you then moved to the private class)

REPOSITORY
  R241 KIO

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

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

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