D22974: Allow usage of QQC2 actions on Kirigami components and now make K.Action based on QQC2.Action

Aleix Pol Gonzalez noreply at phabricator.kde.org
Tue Aug 6 18:22:34 BST 2019


apol added a comment.


  WRT the two icons, maybe it would make sense to populate only the bits we want instead of using the action property? i.e. we can keep a kirigamiAction property for now. This patch is far too big as is already. I would even suggest doing the Kirigami.Action rebase first and then do the other one in different commits.

INLINE COMMENTS

> Action.qml:76
>       */
> -    property ActionIconGroup icon: ActionIconGroup {
> -        id: iconGroup
> -    }
> +//     property ActionIconGroup icon: ActionIconGroup {
> +//         id: iconGroup

Can you check that we're not missing some API? is ActionIconGroup a subset of what Qt offers?

> ActionToolBar.qml:184
>                  itemDelegate: ActionMenuItem {
> -                    visible: actionsLayout.findIndex(actionsLayout.overflowSet, function(act) {return act === ourAction}) > -1 && (ourAction.visible === undefined || ourAction.visible)
> +					visible: actionsLayout.findIndex(actionsLayout.overflowSet, function(act) {return act === ourAction}) > -1 &&  (!ourAction.hasOwnProperty("visible") || ourAction.visible === undefined || ourAction.visible)
>                  }

If `ourAction` doesn't have a visible property, won't `ourAction.visible` be undefined?

> PrivateActionToolButton.qml:42
> +    flat: !control.action || !control.action.icon.color.a
>      //TODO: replace with upstream action when we depend on Qt 5.10
> +//     property Action action

Remove TODO?

> PrivateActionToolButton.qml:89
> +                source: control.action ? (control.action.icon ? control.action.icon.name : control.action.iconName) : ""
> +                visible: control.action && control.action.iconName != "" && control.display != Controls.ToolButton.TextOnly
> +                color: control.flat && control.action && control.action.icon && control.action.icon.color.a > 0 ? control.action.icon.color : label.color

Wouldn't it be control.action.icon.name?

REPOSITORY
  R169 Kirigami

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

To: camiloh, #kirigami, mart
Cc: apol, plasma-devel, fbampaloukas, domson, dkardarakos, davidedmundson, mart, hein
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190806/3ba1c047/attachment-0001.html>


More information about the Plasma-devel mailing list