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
