Skip to content

Simplify ToggleActionMenu and improve its documentation

KActionMenu current state

When a KActionMenu is plugged in a toolbar, the "main" action (the toolbar button itself):

  • cannot be clicked, if setDelayed(false) is called (equivalent to QToolButton::InstantPopup mode). In this case activating the main action just shows the submenu
  • can be clicked (and triggered) if setDelayed(true) is called (i.e. equivalent to QToolButton::DelayedPopup). In this case the main action can trigger its own events.

ToggleActionMenu outline

Simone: From my understanding/ideas ToggleActionMenu should (as already discussed in https://phabricator.kde.org/D21971#559080):

  • Behave as KActionMenu when plugged in a QMenu
  • Make the main action toggleable when plugged in a toolbar
  • Make the main action always be one of the actions in its submenu. This means that when an action in its submenu is triggered it becomes the main action. The main action cannot have its own meaning (as it happens in KActionMenu).

To sum this up in a phrase "When plugged in a toolbar ToggleActionMenu displays a toggleable action among those present in its menu/submenus, when plugged in a QMenu it behaves as a KActionMenu"

API suggestions

Simone: I still think that the API is too complicated, and there are too many cases. I suggest to simplify the class as follows:

  • Remove the logic argument and make ImplicitDefaultAction mode the only mode.
  • Remove suggestDefaultAction and set the default action to the first action added using addAction. Then the developer can change it by calling setDefaultAction.
  • Forbid QToolButton::InstantPopup mode because it does not make sense (how? create a new enum possibly)

In this way the class would cover the use cases for which it is currently used in Okular without adding unuseful complexity.

Open discussion points:

David:

  • When making ImplicitDefaultAction the only operating mode, should pure setDefaultAction() functionality be available in a parent class?
  • How should the menu select the default action? Options:
    • Triggered actions become default action when they are checkable.
    • Triggered actions become default action when they were not excluded with dontUseAsDefaultAction().

Use case Color Mode menu

The color mode menu contains an exclusive list of color mode actions, including “Normal Colors”, and additionally a configuration action.

The toolbar button should always show the last used color mode (i. e. anything except “Normal Colors”), so the feature can be turned on and off by simply clicking the toolbar button.

This means there are two actions which shall not become the default action: “Normal Colors“ (checkable) and “Configure...” (not checkable).

David: This leaves me two options from “open discussion points”:

  • dontUseAsDefaultAction()
  • A parent class implementing only setDefaultAction(), so I can implement my own intelligence.

Virtual meeting results

!383 (merged) tries to minimize the complexity of ToggleActionMenu.

Edited by Laura David Hurka