D27653: Fix ActionToolBar/PrivateActionToolButton in combination with QQC2 Action

[Arjen Hiemstra]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:26f942fc335f: Fix ActionToolBar/PrivateActionToolButton 
in combination with QQC2 Action (authored by ahiemstra).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27653?vs=76370=76536

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

AFFECTED FILES
  src/controls/private/ActionToolBarLayoutDetails.qml
  src/controls/private/PrivateActionToolButton.qml

To: ahiemstra, #kirigami, mart
Cc: camiloh, ognarb, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ngraham, apol, ahiemstra, davidedmundson, mart, hein

D27653: Fix ActionToolBar/PrivateActionToolButton in combination with QQC2 Action

[Arjen Hiemstra]

ahiemstra added inline comments.

INLINE COMMENTS

> ognarb wrote in ActionToolBarLayoutDetails.qml:126
> I think this can be simplified to
> 
>   return !modelData.hasOwnProperty("displayHint") || 
> !modelData.displayHintSet(Kirigami.Action.DisplayHint.AlwaysHide;

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.

REPOSITORY
  R169 Kirigami

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

To: ahiemstra, #kirigami
Cc: camiloh, ognarb, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ngraham, apol, ahiemstra, davidedmundson, mart, hein

D27653: Fix ActionToolBar/PrivateActionToolButton in combination with QQC2 Action

[Camilo Higuita]

camiloh added a comment.


  works okay for me with qqc2 actions

REPOSITORY
  R169 Kirigami

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

To: ahiemstra, #kirigami
Cc: camiloh, ognarb, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ngraham, apol, ahiemstra, davidedmundson, mart, hein

D27653: Fix ActionToolBar/PrivateActionToolButton in combination with QQC2 Action

[Carl Schwan]

ognarb added inline comments.

INLINE COMMENTS

> ActionToolBarLayoutDetails.qml:126
> +
> +if (modelData.hasOwnProperty("displayHint") &&
> +
> modelData.displayHintSet(Kirigami.Action.DisplayHint.AlwaysHide)) {

I think this can be simplified to

  return !modelData.hasOwnProperty("displayHint") || 
!modelData.displayHintSet(Kirigami.Action.DisplayHint.AlwaysHide;

> ActionToolBarLayoutDetails.qml:180
> +}
> +if (modelData.hasOwnProperty("displayHint")
> +&& 
> modelData.displayHintSet(Kirigami.Action.DisplayHint.KeepVisible)) {

same here

REPOSITORY
  R169 Kirigami

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

To: ahiemstra, #kirigami
Cc: ognarb, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, 
apol, ahiemstra, davidedmundson, mart, hein

D27653: Fix ActionToolBar/PrivateActionToolButton in combination with QQC2 Action

[Arjen Hiemstra]

ahiemstra created this revision.
ahiemstra added a reviewer: Kirigami.
Herald added a project: Kirigami.
Herald added a subscriber: plasma-devel.
ahiemstra requested review of this revision.

REVISION 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)

REPOSITORY
  R169 Kirigami

BRANCH
  actiontoolbar_action

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

AFFECTED FILES
  src/controls/private/ActionToolBarLayoutDetails.qml
  src/controls/private/PrivateActionToolButton.qml

To: ahiemstra, #kirigami
Cc: plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, ngraham, apol, 
ahiemstra, davidedmundson, mart, hein