Merge lp:~tpeeters/ubuntu-ui-toolkit/ToolbarButton-disable into lp:ubuntu-ui-toolkit
- ToolbarButton-disable
- Merge into trunk
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 | ||||
Related bugs: |
|
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.
Description of the change
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:649
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
Tim Peeters (tpeeters) wrote : | # |
the trigger() of the action is defined by the developers, so we cannot force that they check it
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().
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:656
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:657
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:659
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
UNSTABLE: http://
Click here to trigger a rebuild:
http://
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".
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
UNSTABLE: http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
UNSTABLE: http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
UNSTABLE: http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
UNSTABLE: http://
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:661
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
UNSTABLE: http://
PS Jenkins bot (ps-jenkins) : | # |
Preview Diff
1 | === modified file 'components.api' | |||
2 | --- components.api 2013-08-16 10:16:48 +0000 | |||
3 | +++ components.api 2013-08-21 17:07:26 +0000 | |||
4 | @@ -16,7 +16,8 @@ | |||
5 | 16 | property Action action | 16 | property Action action |
6 | 17 | property string text | 17 | property string text |
7 | 18 | property url iconSource | 18 | property url iconSource |
9 | 19 | signal triggered(var caller) | 19 | signal triggered(var value) |
10 | 20 | function trigger(value) | ||
11 | 20 | modules/Ubuntu/Components/ActionList.qml | 21 | modules/Ubuntu/Components/ActionList.qml |
12 | 21 | QtObject | 22 | QtObject |
13 | 22 | default property list<Action> children | 23 | default property list<Action> children |
14 | 23 | 24 | ||
15 | === modified file 'modules/Ubuntu/Components/AbstractButton.qml' | |||
16 | --- modules/Ubuntu/Components/AbstractButton.qml 2013-06-12 18:22:07 +0000 | |||
17 | +++ modules/Ubuntu/Components/AbstractButton.qml 2013-08-21 17:07:26 +0000 | |||
18 | @@ -47,7 +47,7 @@ | |||
19 | 47 | /*! | 47 | /*! |
20 | 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. |
21 | 49 | */ | 49 | */ |
23 | 50 | onClicked: button.triggered(button) | 50 | onClicked: button.trigger() |
24 | 51 | 51 | ||
25 | 52 | Keys.onEnterPressed: clicked() | 52 | Keys.onEnterPressed: clicked() |
26 | 53 | Keys.onReturnPressed: clicked() | 53 | Keys.onReturnPressed: clicked() |
27 | 54 | 54 | ||
28 | === modified file 'modules/Ubuntu/Components/ActionItem.qml' | |||
29 | --- modules/Ubuntu/Components/ActionItem.qml 2013-07-01 22:13:31 +0000 | |||
30 | +++ modules/Ubuntu/Components/ActionItem.qml 2013-08-21 17:07:26 +0000 | |||
31 | @@ -59,10 +59,18 @@ | |||
32 | 59 | /*! | 59 | /*! |
33 | 60 | Called when the actionItem is triggered. | 60 | Called when the actionItem is triggered. |
34 | 61 | */ | 61 | */ |
36 | 62 | signal triggered(var caller) | 62 | signal triggered(var value) |
37 | 63 | 63 | ||
38 | 64 | /*! | 64 | /*! |
40 | 65 | If \l action is set, this will call action.trigger(caller). | 65 | If \l action is set, this will trigger it. |
41 | 66 | */ | 66 | */ |
43 | 67 | onTriggered: if (action) action.triggered(caller) | 67 | onTriggered: if (action) action.trigger(value) |
44 | 68 | |||
45 | 69 | /*! | ||
46 | 70 | Trigger this action item if it is enabled. | ||
47 | 71 | */ | ||
48 | 72 | function trigger(value) { | ||
49 | 73 | var passingValue = value ? value : null | ||
50 | 74 | if (actionItem.enabled) actionItem.triggered(passingValue); | ||
51 | 75 | } | ||
52 | 68 | } | 76 | } |
53 | 69 | 77 | ||
54 | === modified file 'modules/Ubuntu/Components/Panel.qml' | |||
55 | --- modules/Ubuntu/Components/Panel.qml 2013-08-15 10:59:06 +0000 | |||
56 | +++ modules/Ubuntu/Components/Panel.qml 2013-08-21 17:07:26 +0000 | |||
57 | @@ -416,17 +416,17 @@ | |||
58 | 416 | // find the first child with a triggered property: | 416 | // find the first child with a triggered property: |
59 | 417 | function getTriggerableItem(mouse) { | 417 | function getTriggerableItem(mouse) { |
60 | 418 | var item = bar; // contains the children | 418 | var item = bar; // contains the children |
62 | 419 | while (item && !item.hasOwnProperty("triggered")) { | 419 | while (item && !item.hasOwnProperty("trigger")) { |
63 | 420 | var coords = mapToItem(item, mouse.x, mouse.y); | 420 | var coords = mapToItem(item, mouse.x, mouse.y); |
64 | 421 | // FIXME: When using a ListView the highlight may be | 421 | // FIXME: When using a ListView the highlight may be |
65 | 422 | // returned instead of the Item that you are looking for | 422 | // returned instead of the Item that you are looking for |
66 | 423 | item = item.childAt(coords.x, coords.y); | 423 | item = item.childAt(coords.x, coords.y); |
67 | 424 | } | 424 | } |
69 | 425 | return item; // will be null if no item has triggered() signal. | 425 | return item; // will be null if no item has trigger() function. |
70 | 426 | } | 426 | } |
71 | 427 | 427 | ||
74 | 428 | // forward clicked and triggered events to any child Item with a | 428 | // forward clicked() and trigger() events to any child Item with a |
75 | 429 | // clicked() or triggered() signal, not | 429 | // clicked() or trigger() function, not |
76 | 430 | // just MouseAreas since MouseAreas would block swiping of the panel. | 430 | // just MouseAreas since MouseAreas would block swiping of the panel. |
77 | 431 | // This must also happen when the panel is locked, so the DraggingArea is | 431 | // This must also happen when the panel is locked, so the DraggingArea is |
78 | 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. |
79 | @@ -437,7 +437,7 @@ | |||
80 | 437 | pressedItem.clicked(); | 437 | pressedItem.clicked(); |
81 | 438 | } else if (pressedItem && pressedItem === getTriggerableItem(mouse)) { | 438 | } else if (pressedItem && pressedItem === getTriggerableItem(mouse)) { |
82 | 439 | // Click event positioned at the Item where the user first pressed | 439 | // Click event positioned at the Item where the user first pressed |
84 | 440 | pressedItem.triggered(pressedItem); | 440 | pressedItem.trigger(); |
85 | 441 | } | 441 | } |
86 | 442 | } | 442 | } |
87 | 443 | 443 | ||
88 | 444 | 444 | ||
89 | === modified file 'modules/Ubuntu/Components/Popups/ActionSelectionPopover.qml' | |||
90 | --- modules/Ubuntu/Components/Popups/ActionSelectionPopover.qml 2013-07-04 21:36:23 +0000 | |||
91 | +++ modules/Ubuntu/Components/Popups/ActionSelectionPopover.qml 2013-08-21 17:07:26 +0000 | |||
92 | @@ -120,6 +120,8 @@ | |||
93 | 120 | } | 120 | } |
94 | 121 | /*! \internal */ | 121 | /*! \internal */ |
95 | 122 | onTriggered: popover.hide() | 122 | onTriggered: popover.hide() |
96 | 123 | visible: enabled | ||
97 | 124 | height: visible ? implicitHeight : 0 | ||
98 | 123 | } | 125 | } |
99 | 124 | 126 | ||
100 | 125 | grabDismissAreaEvents: false | 127 | grabDismissAreaEvents: false |
101 | 126 | 128 | ||
102 | === modified file 'modules/Ubuntu/Components/TextInputPopover.qml' | |||
103 | --- modules/Ubuntu/Components/TextInputPopover.qml 2013-02-13 09:36:45 +0000 | |||
104 | +++ modules/Ubuntu/Components/TextInputPopover.qml 2013-08-21 17:07:26 +0000 | |||
105 | @@ -22,38 +22,38 @@ | |||
106 | 22 | actions: ActionList { | 22 | actions: ActionList { |
107 | 23 | Action { | 23 | Action { |
108 | 24 | text: i18n.tr("Select All") | 24 | text: i18n.tr("Select All") |
111 | 25 | visible: target && target.text !== "" && target.text !== target.selectedText | 25 | enabled: target && target.text !== "" && target.text !== target.selectedText |
112 | 26 | onTriggered: caller.selectAll() | 26 | onTriggered: target.selectAll() |
113 | 27 | } | 27 | } |
114 | 28 | Action { | 28 | Action { |
115 | 29 | text: i18n.tr("Select Word") | 29 | text: i18n.tr("Select Word") |
118 | 30 | visible: target && target.text !== "" && target.selectedText === "" | 30 | enabled: target && target.text !== "" && target.selectedText === "" |
119 | 31 | onTriggered: caller.selectWord() | 31 | onTriggered: target.selectWord() |
120 | 32 | } | 32 | } |
121 | 33 | Action { | 33 | Action { |
122 | 34 | text: i18n.tr("Cut") | 34 | text: i18n.tr("Cut") |
125 | 35 | visible: target && target.selectedText !== "" | 35 | enabled: target && target.selectedText !== "" |
126 | 36 | onTriggered: caller.cut() | 36 | onTriggered: target.cut() |
127 | 37 | } | 37 | } |
128 | 38 | Action { | 38 | Action { |
129 | 39 | text: i18n.tr("Copy") | 39 | text: i18n.tr("Copy") |
132 | 40 | visible: target && target.selectedText !== "" | 40 | enabled: target && target.selectedText !== "" |
133 | 41 | onTriggered: caller.copy() | 41 | onTriggered: target.copy() |
134 | 42 | } | 42 | } |
135 | 43 | Action { | 43 | Action { |
136 | 44 | text: i18n.tr("Paste") | 44 | text: i18n.tr("Paste") |
139 | 45 | visible: target && target.canPaste | 45 | enabled: target && target.canPaste |
140 | 46 | onTriggered: caller.paste() | 46 | onTriggered: target.paste() |
141 | 47 | } | 47 | } |
142 | 48 | Action { | 48 | Action { |
143 | 49 | text: i18n.tr("Undo") | 49 | text: i18n.tr("Undo") |
146 | 50 | visible: target && target.canUndo | 50 | enabled: target && target.canUndo |
147 | 51 | onTriggered: caller.undo() | 51 | onTriggered: target.undo() |
148 | 52 | } | 52 | } |
149 | 53 | Action { | 53 | Action { |
150 | 54 | text: i18n.tr("Redo") | 54 | text: i18n.tr("Redo") |
153 | 55 | visible: target && target.canRedo | 55 | enabled: target && target.canRedo |
154 | 56 | onTriggered: caller.redo() | 56 | onTriggered: target.redo() |
155 | 57 | } | 57 | } |
156 | 58 | } | 58 | } |
157 | 59 | } | 59 | } |
158 | 60 | 60 | ||
159 | === modified file 'tests/resources/toolbar/toolbar.qml' | |||
160 | --- tests/resources/toolbar/toolbar.qml 2013-06-14 16:28:04 +0000 | |||
161 | +++ tests/resources/toolbar/toolbar.qml 2013-08-21 17:07:26 +0000 | |||
162 | @@ -47,6 +47,7 @@ | |||
163 | 47 | iconSource: Qt.resolvedUrl("../../../examples/ubuntu-ui-toolkit-gallery/small_avatar.png") | 47 | iconSource: Qt.resolvedUrl("../../../examples/ubuntu-ui-toolkit-gallery/small_avatar.png") |
164 | 48 | text: "oh" | 48 | text: "oh" |
165 | 49 | onTriggered: print("lala") | 49 | onTriggered: print("lala") |
166 | 50 | enabled: false | ||
167 | 50 | } | 51 | } |
168 | 51 | ToolbarButton { | 52 | ToolbarButton { |
169 | 52 | action: action1 | 53 | action: action1 |
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.