D21971: Replace ToolAction by ToggleActionMenu

[Simone Gaiarin]

simgunz added a comment.


  To sum it up, if I understand correctly:
  
  - you agree in removing `DefaultLogic`
  - we can remove `InstantPopup` given that we can use `KActionMenu` or `KSelectAction` to provide that use case (so no need to raise exceptions)
  
  The change to `ToggleActionMenu::defaultAction` is required, otherwise if `setDefaultAction` is called on action1 manually and then action2 is checked, `defaultAction` returns action1.
  
  > Should be probably ok, that’s just not really “implicit”.
  
  In this case `setDefaultAction` has the meaning of `suggestDefaultAction`, i.e., I set the `textSelectionAction` as a default, but then if any of the other actions are checked the default action is changed.
  An alternative is to keep `suggestDefaultAction` public and make `setDefaultAction` private. (even better I would rename `suggestDefaultAction` > `setDefaultAction` and `setDefaultAction` > `something else`)
  
  ----
  
  Thinking forward, `ToggleActionMenu` could become just a 'mode' of `KActionMenu`. Right?

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/20191107/8535acc5/attachment.html>