Differential D27653

Fix ActionToolBar/PrivateActionToolButton in combination with QQC2 Action
ClosedPublic
Actions

Authored by ahiemstra on Feb 25 2020, 2:54 PM.

Details

Reviewers
mart
Group Reviewers
Kirigami
Commits
R169:26f942fc335f: Fix ActionToolBar/PrivateActionToolButton in combination with QQC2 Action
Summary

This applies a few fixes to ActionToolBar and PrivateActionToolButton when
used in combination with QtQuick Controls 2's Action instead of
Kirigami.Action. It is a more elaborate version of D27638 .

  • Properly check if PrivateActionToolButton should be visible
  • Use hasOwnProperty for checking property exists and write out everything
  • Don't try to use tooltip property that may not be defined
Test Plan

Kirigami Gallery still shows actions correctly.
A test QML with QQC2 actions shows them correct.

(I have an autotest for ActionToolBar in the works that will include a
tests with QQC2 Action)

Diff Detail

Repository
R169 Kirigami
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ahiemstra created this revision.Feb 25 2020, 2:54 PM
Restricted Application added a project: Kirigami. · View Herald TranscriptFeb 25 2020, 2:54 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ahiemstra requested review of this revision.Feb 25 2020, 2:54 PM
ognarb added a subscriber: ognarb.Feb 25 2020, 3:33 PM
ognarb added inline comments.
src/controls/private/ActionToolBarLayoutDetails.qml
126 ↗(On Diff #76370)

I think this can be simplified to

return !modelData.hasOwnProperty("displayHint") || !modelData.displayHintSet(Kirigami.Action.DisplayHint.AlwaysHide;
180 ↗(On Diff #76370)

same here

camiloh added a subscriber: camiloh.Feb 25 2020, 5:09 PM

works okay for me with qqc2 actions

ahiemstra added inline comments.Feb 26 2020, 1:24 PM
src/controls/private/ActionToolBarLayoutDetails.qml
126 ↗(On Diff #76370)

The reason I'm expanding these in the first place is for readability, I keep finding the simplified forms hard to read as soon as there's more than one check involved. So I would rather not simplify this, as now the logic is very explicit and does not need a lot of mental parsing to figure out what is going on.

mart accepted this revision.Feb 26 2020, 1:29 PM
This revision is now accepted and ready to land.Feb 26 2020, 1:29 PM
Closed by commit R169:26f942fc335f: Fix ActionToolBar/PrivateActionToolButton in combination with QQC2 Action (authored by ahiemstra). · Explain WhyFeb 27 2020, 12:16 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 76536

View Options

src/controls/private/ActionToolBarLayoutDetails.qml

Loading...
View Options

src/controls/private/PrivateActionToolButton.qml

Loading...