D21971: Replace ToolAction by ToggleActionMenu

[David Hurka]

davidhurka added a comment.


  In D21971#559485 <https://phabricator.kde.org/D21971#559485>, @simgunz wrote:
  
  > To sum it up, if I understand correctly:
  >
  > - we can remove `InstantPopup` given that we can use `KActionMenu` or `KSelectAction` to provide that use case (so no need to raise exceptions)
  
  
  Yes, but how? It can’t simply be forbidden in the constructor argument list. Remove it from the body of the constructor, so KActionMenu is set to MenuButtonPopup instead?
  
  > - you agree in removing `DefaultLogic`
  
  No, it is needed when other actions than the exclusive group are added to the menu.
  Example is D21195 <https://phabricator.kde.org/D21195>, where an action to configure the color modes is added to the Color Mode menu. When you click that action in ImplicitDefaultAction mode, it would become the default action, and the toolbar tells you that you are in color mode “Configure...”.
  Other option is to allow only checkable actions to become the default action. But then, you can’t add real “actions” (opposed to “tools”). Imagine the user has some construction plan, and the application has several actions of kind “Mark for <job>”, and the user wants to apply one of these action to several items. Then it could be useful to collect all “Mark for <job>” actions in a ToggleActionMenu ImplicitDefaultAction mode, while the actions can not be checked.
  
  > The change to `ToggleActionMenu::defaultAction` is required, otherwise if `setDefaultAction` is called on action1 manually and then action2 is checked, `defaultAction` returns action1.
  
  I can’t follow you here. Yes, when in DefaultLogic mode, and setDefaultAction(action1) was called, defaultAction() returns action1. Maybe your thinking was focused on checkable actions so far?
  
  > [...]
  >  Thinking forward, `ToggleActionMenu` could become just a 'mode' of `KActionMenu`. Right?
  
  Yes. But I would like Bug 413827 <https://bugs.kde.org/show_bug.cgi?id=413827> to be resolved (so PopupMode enumerator is used).

REPOSITORY
  R223 Okular

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

To: davidhurka
Cc: ngraham, simgunz, okular-devel, johnzh, andisa, siddharthmanthan, maguirre, fbampaloukas, joaonetto, kezik, tfella, darcyshen, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20191109/7f63cc0f/attachment-0001.html>