D25660: Decouple KBookmarksMenu from KActionCollection

David Faure noreply at phabricator.kde.org
Sun Jan 12 11:56:56 GMT 2020


dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Thanks for the patch.
  
  Please call setObjectName() on the actions so that their object name is fixed (no matter which ctor is used).  (this is because KActionCollection::addAction calls setObjectName on the actions)
  
  This can even help apps to just get all 3 actions and put them into an action collection without having to copy the standard names from the documentation.
  
    QAction *addAction = menu->addBookmarkAction();
    actionCollection()->addAction(addAction->objectName(), addAction);
  
  Maybe you can use the C++11 feature of one ctor calling another to avoid the setup() method which will look even more strange once the deprecated ctor is removed.
  
  The @since value needs to be updated obviously.

INLINE COMMENTS

> kbookmarkmenu.cpp:347
> +
> +        if (m_actionCollection) {
> +            m_actionCollection->addAction(QStringLiteral("add_bookmark"), d->addAddBookmark);

The m_bIsRoot check disappeared in the refactoring!

Actions in submenus should not be named.

Same thing for the action named add_bookmarks_list above.

REPOSITORY
  R294 KBookmarks

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

To: nicolasfella, #frameworks, dfaure
Cc: dfaure, apol, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20200112/330314bf/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list