Merge lp:~tpeeters/ubuntu-ui-toolkit/ToolbarButton-disable into lp:ubuntu-ui-toolkit

Proposed by Tim Peeters
Status: Merged
Approved by: Tim Peeters
Approved revision: 661
Merged at revision: 707
Proposed branch: lp:~tpeeters/ubuntu-ui-toolkit/ToolbarButton-disable
Merge into: lp:ubuntu-ui-toolkit
Diff against target: 169 lines (+36/-24)
7 files modified
components.api (+2/-1)
modules/Ubuntu/Components/AbstractButton.qml (+1/-1)
modules/Ubuntu/Components/ActionItem.qml (+11/-3)
modules/Ubuntu/Components/Panel.qml (+5/-5)
modules/Ubuntu/Components/Popups/ActionSelectionPopover.qml (+2/-0)
modules/Ubuntu/Components/TextInputPopover.qml (+14/-14)
tests/resources/toolbar/toolbar.qml (+1/-0)
To merge this branch: bzr merge lp:~tpeeters/ubuntu-ui-toolkit/ToolbarButton-disable
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Cris Dywan Approve
Review via email: mp+176395@code.launchpad.net

Commit message

Do not trigger a ToolbarButton that is not enabled.

To post a comment you must log in.
Revision history for this message
Cris Dywan (kalikiana) wrote :

I'm thinking we should have a "trigger()" method that does this check. The AbstractButton.clicked also doesn't check enabled. And adding this in apps is even more error-prone.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Tim Peeters (tpeeters) wrote :

the trigger() of the action is defined by the developers, so we cannot force that they check it

Revision history for this message
Tim Peeters (tpeeters) wrote :

correction: triggered() is defined by the developers.

Unity.Action has a trigger() that does the check, need to change the UITK code to call trigger() instead of triggered().

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Cris Dywan (kalikiana) wrote :

Nice stuff, thanks for making the changes. And good to have the value argument retained: this came up only in IRC, so I'm mentioning it for clarity, I was running into issues in another place because the value was "hidden".

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'components.api'
--- components.api 2013-08-16 10:16:48 +0000
+++ components.api 2013-08-21 17:07:26 +0000
@@ -16,7 +16,8 @@
16 property Action action16 property Action action
17 property string text17 property string text
18 property url iconSource18 property url iconSource
19 signal triggered(var caller)19 signal triggered(var value)
20 function trigger(value)
20modules/Ubuntu/Components/ActionList.qml21modules/Ubuntu/Components/ActionList.qml
21QtObject22QtObject
22 default property list<Action> children23 default property list<Action> children
2324
=== modified file 'modules/Ubuntu/Components/AbstractButton.qml'
--- modules/Ubuntu/Components/AbstractButton.qml 2013-06-12 18:22:07 +0000
+++ modules/Ubuntu/Components/AbstractButton.qml 2013-08-21 17:07:26 +0000
@@ -47,7 +47,7 @@
47 /*!47 /*!
48 If a button is clicked, its triggered() signal will automatically be called.48 If a button is clicked, its triggered() signal will automatically be called.
49 */49 */
50 onClicked: button.triggered(button)50 onClicked: button.trigger()
5151
52 Keys.onEnterPressed: clicked()52 Keys.onEnterPressed: clicked()
53 Keys.onReturnPressed: clicked()53 Keys.onReturnPressed: clicked()
5454
=== modified file 'modules/Ubuntu/Components/ActionItem.qml'
--- modules/Ubuntu/Components/ActionItem.qml 2013-07-01 22:13:31 +0000
+++ modules/Ubuntu/Components/ActionItem.qml 2013-08-21 17:07:26 +0000
@@ -59,10 +59,18 @@
59 /*!59 /*!
60 Called when the actionItem is triggered.60 Called when the actionItem is triggered.
61 */61 */
62 signal triggered(var caller)62 signal triggered(var value)
6363
64 /*!64 /*!
65 If \l action is set, this will call action.trigger(caller).65 If \l action is set, this will trigger it.
66 */66 */
67 onTriggered: if (action) action.triggered(caller)67 onTriggered: if (action) action.trigger(value)
68
69 /*!
70 Trigger this action item if it is enabled.
71 */
72 function trigger(value) {
73 var passingValue = value ? value : null
74 if (actionItem.enabled) actionItem.triggered(passingValue);
75 }
68}76}
6977
=== modified file 'modules/Ubuntu/Components/Panel.qml'
--- modules/Ubuntu/Components/Panel.qml 2013-08-15 10:59:06 +0000
+++ modules/Ubuntu/Components/Panel.qml 2013-08-21 17:07:26 +0000
@@ -416,17 +416,17 @@
416 // find the first child with a triggered property:416 // find the first child with a triggered property:
417 function getTriggerableItem(mouse) {417 function getTriggerableItem(mouse) {
418 var item = bar; // contains the children418 var item = bar; // contains the children
419 while (item && !item.hasOwnProperty("triggered")) {419 while (item && !item.hasOwnProperty("trigger")) {
420 var coords = mapToItem(item, mouse.x, mouse.y);420 var coords = mapToItem(item, mouse.x, mouse.y);
421 // FIXME: When using a ListView the highlight may be421 // FIXME: When using a ListView the highlight may be
422 // returned instead of the Item that you are looking for422 // returned instead of the Item that you are looking for
423 item = item.childAt(coords.x, coords.y);423 item = item.childAt(coords.x, coords.y);
424 }424 }
425 return item; // will be null if no item has triggered() signal.425 return item; // will be null if no item has trigger() function.
426 }426 }
427427
428 // forward clicked and triggered events to any child Item with a428 // forward clicked() and trigger() events to any child Item with a
429 // clicked() or triggered() signal, not429 // clicked() or trigger() function, not
430 // just MouseAreas since MouseAreas would block swiping of the panel.430 // just MouseAreas since MouseAreas would block swiping of the panel.
431 // This must also happen when the panel is locked, so the DraggingArea is431 // This must also happen when the panel is locked, so the DraggingArea is
432 // never disabled, and other signal handlers will return when panel.locked is true.432 // never disabled, and other signal handlers will return when panel.locked is true.
@@ -437,7 +437,7 @@
437 pressedItem.clicked();437 pressedItem.clicked();
438 } else if (pressedItem && pressedItem === getTriggerableItem(mouse)) {438 } else if (pressedItem && pressedItem === getTriggerableItem(mouse)) {
439 // Click event positioned at the Item where the user first pressed439 // Click event positioned at the Item where the user first pressed
440 pressedItem.triggered(pressedItem);440 pressedItem.trigger();
441 }441 }
442 }442 }
443443
444444
=== modified file 'modules/Ubuntu/Components/Popups/ActionSelectionPopover.qml'
--- modules/Ubuntu/Components/Popups/ActionSelectionPopover.qml 2013-07-04 21:36:23 +0000
+++ modules/Ubuntu/Components/Popups/ActionSelectionPopover.qml 2013-08-21 17:07:26 +0000
@@ -120,6 +120,8 @@
120 }120 }
121 /*! \internal */121 /*! \internal */
122 onTriggered: popover.hide()122 onTriggered: popover.hide()
123 visible: enabled
124 height: visible ? implicitHeight : 0
123 }125 }
124126
125 grabDismissAreaEvents: false127 grabDismissAreaEvents: false
126128
=== modified file 'modules/Ubuntu/Components/TextInputPopover.qml'
--- modules/Ubuntu/Components/TextInputPopover.qml 2013-02-13 09:36:45 +0000
+++ modules/Ubuntu/Components/TextInputPopover.qml 2013-08-21 17:07:26 +0000
@@ -22,38 +22,38 @@
22 actions: ActionList {22 actions: ActionList {
23 Action {23 Action {
24 text: i18n.tr("Select All")24 text: i18n.tr("Select All")
25 visible: target && target.text !== "" && target.text !== target.selectedText25 enabled: target && target.text !== "" && target.text !== target.selectedText
26 onTriggered: caller.selectAll()26 onTriggered: target.selectAll()
27 }27 }
28 Action {28 Action {
29 text: i18n.tr("Select Word")29 text: i18n.tr("Select Word")
30 visible: target && target.text !== "" && target.selectedText === ""30 enabled: target && target.text !== "" && target.selectedText === ""
31 onTriggered: caller.selectWord()31 onTriggered: target.selectWord()
32 }32 }
33 Action {33 Action {
34 text: i18n.tr("Cut")34 text: i18n.tr("Cut")
35 visible: target && target.selectedText !== ""35 enabled: target && target.selectedText !== ""
36 onTriggered: caller.cut()36 onTriggered: target.cut()
37 }37 }
38 Action {38 Action {
39 text: i18n.tr("Copy")39 text: i18n.tr("Copy")
40 visible: target && target.selectedText !== ""40 enabled: target && target.selectedText !== ""
41 onTriggered: caller.copy()41 onTriggered: target.copy()
42 }42 }
43 Action {43 Action {
44 text: i18n.tr("Paste")44 text: i18n.tr("Paste")
45 visible: target && target.canPaste45 enabled: target && target.canPaste
46 onTriggered: caller.paste()46 onTriggered: target.paste()
47 }47 }
48 Action {48 Action {
49 text: i18n.tr("Undo")49 text: i18n.tr("Undo")
50 visible: target && target.canUndo50 enabled: target && target.canUndo
51 onTriggered: caller.undo()51 onTriggered: target.undo()
52 }52 }
53 Action {53 Action {
54 text: i18n.tr("Redo")54 text: i18n.tr("Redo")
55 visible: target && target.canRedo55 enabled: target && target.canRedo
56 onTriggered: caller.redo()56 onTriggered: target.redo()
57 }57 }
58 }58 }
59}59}
6060
=== modified file 'tests/resources/toolbar/toolbar.qml'
--- tests/resources/toolbar/toolbar.qml 2013-06-14 16:28:04 +0000
+++ tests/resources/toolbar/toolbar.qml 2013-08-21 17:07:26 +0000
@@ -47,6 +47,7 @@
47 iconSource: Qt.resolvedUrl("../../../examples/ubuntu-ui-toolkit-gallery/small_avatar.png")47 iconSource: Qt.resolvedUrl("../../../examples/ubuntu-ui-toolkit-gallery/small_avatar.png")
48 text: "oh"48 text: "oh"
49 onTriggered: print("lala")49 onTriggered: print("lala")
50 enabled: false
50 }51 }
51 ToolbarButton {52 ToolbarButton {
52 action: action153 action: action1

Subscribers

People subscribed via source and target branches

to status/vote changes: