[DEMO] Enhance ToggleActionMenu with ImplicitDefaultAction mode.
Needs ReviewPublic

Authored by davidhurka on Jun 21 2019, 7:08 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

(This is not a merge request.)

This allows to use ToggleActionMenu as nearly drop-in replacement for ToolAction,
without additional code statements.

ImplicitDefaultAction mode means that the default action of the toolbar buttons
is determined by the last triggered action,
or the first checked action in the menu.

This patch is not fully tested.

@simgunz asked about a functionality somewhere near this in D21755#482896

Diff Detail

Repository
R223 Okular
Branch
create-configurable-toggleactionmenu
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 13103
Build 13121: arc lint + arc unit
davidhurka requested review of this revision.Jun 21 2019, 7:08 PM
davidhurka created this revision.

I did test this today and it seems to be working correctly, at least in the use cases of Okular, i.e. for select annotation tools and for the annotation toolbar. Actually I am currently using in D15580 now (will push soon), given that ToolAction was not enough for what I needed ( I'll update my code accordingly if this revision is modified).

Suggested changes:

  • Make QToolButton::MenuButtonPopup, and ToggleActionMenu::ImplicitDefaultAction the defaults in the constructor so we can call it as d->aMouseModeMenu = new ToggleActionMenu( QIcon(),QString(), this);
  • I do not fully understand the purpose of the method setSuggestedDefaultAction. I would remove it and leave only setDefaultAction.

This is how I am currently using it:

ToggleActionMenu *ta = new ToggleActionMenu( QIcon(),QString(), this, QToolButton::MenuButtonPopup, ToggleActionMenu::ImplicitDefaultAction );
connect(ta, &ToggleActionMenu::toggled, this, &AnnotationActionHandler::setTextToolsEnabled);
ac->addAction( QStringLiteral("annotation_geometrical_shape"), ta );
ta->setText( i18nc( "@action", "Geometrical shapes" ) );
ta->addAction( annArrow );
ta->addAction( annStraightLine );
ta->addAction( annRectangle );
ta->setDefaultAction( annArrow );

I am only considering the use cases of Okular.

@simgunz Thanks for your feedback. I have just got some more private to-do, so if I do not look at your feedback later today, please wait at least until wednesday. :)

Suggested changes:

  • Make QToolButton::MenuButtonPopup, and ToggleActionMenu::ImplicitDefaultAction the defaults in the constructor so we can call it as d->aMouseModeMenu = new ToggleActionMenu( QIcon(),QString(), this);

That would make the longer way necessary for my own ideas, where I have put “Configure...” into the Color Mode menu.

  • I do not fully understand the purpose of the method setSuggestedDefaultAction. I would remove it and leave only setDefaultAction.

I don’t think the ImplicitDefaultAction mode makes sense without it. For the mouse mode menu (as shown in this patch), it is needed, because the default action should be Text Selection, because that mode is most likely needed. But it is possible that Area Selection is already checked, and in that case Area Selection should be shown on the toolbar button.

Or do you mean ImplicitDefaultAction mode should change the behaviour of setDefaultAction(), so it only suggests the default action? I don’t think that’s good, because it will not really “set” then. Additionally, toggleactionmenu.cpp:57 would not work like it is done now.

This is how I am currently using it: [...]

Yeah, in that case I would recommend suggestDefaultAction(), for the case that annStraightLine is already checked.