Unity

Merge lp:~macslow/unity/phablet-snap-decision-action-expansion into lp:unity/phablet

  1. phablet-snap-decision-action-expansion
  2. Merge into phablet
Proposed by Mirco Müller
Status: Work in progress
Proposed branch: lp:~macslow/unity/phablet-snap-decision-action-expansion
Merge into: lp:unity/phablet
Diff against target: 343 lines (+160/-57)
4 files modified
Notifications/Notification.qml (+104/-14)
Notifications/Notifications.qml (+10/-13)
Notifications/timings.js (+0/-23)
tests/qmltests/Notifications/tst_Notifications.qml (+46/-7)
To merge this branch: bzr merge lp:~macslow/unity/phablet-snap-decision-action-expansion
Reviewer Review Type Date Requested Status
Michał Sawicz Needs Fixing
PS Jenkins bot (community) continuous-integration Needs Fixing
Michael Zanetti (community) Needs Fixing
Review via email: mp+165370@code.launchpad.net

Commit message

Added support and (stand-alone and interactive) tests for more than 2 actions passed to snap-decisions.

Description of the change

Added support and (stand-alone and interactive) tests for more than 2 actions passed to snap-decisions.

To post a comment you must log in.
lp:~macslow/unity/phablet-snap-decision-action-expansion updated
687. By Mirco Müller

Got rid of custom timings.js and using UbuntuAnimation instead

688. By Mirco Müller

re-merged with trunk

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:686
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~macslow/unity/phablet-snap-decision-action-expansion/+merge/165370/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/1015/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/994
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/890
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/890/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/894
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1638

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/1015/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
Michał Sawicz (saviq) wrote :

71 + property real buttonWidth: (width - units.gu(1)) / 2

103 + leftMargin: units.gu(1)

118 + spacing: units.gu(1)

Please use contentColumn.spacing here

79 + height: units.gu(4)

107 + height: units.gu(4)

Please use anchors instead.

90 + NumberAnimation {
91 + duration: UbuntuAnimation.SnapDuration
92 + easing: UbuntuAnimation.StandardEasing
93 + }

193 - duration: Timings.snapBeat
194 - easing.type: Timings.easing
195 + duration: UbuntuAnimation.SnapDuration
196 + easing: UbuntuAnimation.StandardEasing

204 - duration: Timings.snapBeat
205 - easing.type: Timings.easing
206 + duration: UbuntuAnimation.SnapDuration
207 + easing: UbuntuAnimation.StandardEasing

215 - duration: Timings.fastBeat
216 - easing.type: Timings.easing
217 + duration: UbuntuAnimation.FastDuration
218 + easing: UbuntuAnimation.StandardEasing

225 - duration: Timings.fastBeat
226 - easing.type: Timings.easing
227 + duration: UbuntuAnimation.FastDuration
228 + easing: UbuntuAnimation.StandardEasing

You should use UbuntuNumberAnimation with the default easing here.

115 + id: buttonColumn
116 +

That's unused.

334 + }
335 + else {

Please use a single line.

329 +
330 + // click to collapse
331 + mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
332 + waitForRendering(notification)
333 + actionSpy.clear()

This seems unused?

339 + actionSpy.clear()

Should probably happen in cleanup()?

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

PASSED: Continuous integration, rev:688
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/1018/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/998
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/893
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/893/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/897
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1642

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/1018/rebuild

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

PASSED: Continuous integration, rev:686
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/1019/
Executed test runs:
    SUCCESS: http://s-jenkins:8080/job/unity-phablet-qmluitests/999
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/894
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/894/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/898
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1643

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/1019/rebuild

review: Approve (continuous-integration)
Reply
lp:~macslow/unity/phablet-snap-decision-action-expansion updated
689. By Mirco Müller

Fixed mentioned issues from MR... improved expansion/collapse test

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:689
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/1029/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/1010
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/904
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/904/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/908
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1660

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/1029/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Replace this:

333 + wait(100)
334 + actionSpy.clear()
335 +
336 + var heightAfterCollapse = notification.height
337 + compare(initialHeight, heightAfterCollapse, "height of collapsed snap-decision is not the same as its initial height")

with this:

tryCompare(notification, "height", initialHeight)

review: Needs Fixing
Reply
lp:~macslow/unity/phablet-snap-decision-action-expansion updated
690. By Mirco Müller

Make height-test after collapsing the snap-decision work.

Revision history for this message
Mirco Müller (macslow) wrote :

Thanks for the tips. I've sorted out all issues and also the height-comparision for the expanding/collapsing snap-decision work now as intended.

Reply
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:690
http://jenkins.qa.ubuntu.com/job/unity-phablet-ci/1033/
Executed test runs:
    UNSTABLE: http://s-jenkins:8080/job/unity-phablet-qmluitests/1015
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/908
        deb: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-armhf-ci/908/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-raring-i386-ci/912
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner/1662

Click here to trigger a rebuild:
http://s-jenkins:8080/job/unity-phablet-ci/1033/rebuild

review: Needs Fixing (continuous-integration)
Reply
Revision history for this message
Michał Sawicz (saviq) wrote :

Failed test:

'height of expanded snap-decision is not the greater than its initial height' returned FALSE. ()

review: Needs Fixing
Reply
lp:~macslow/unity/phablet-snap-decision-action-expansion updated
691. By Mirco Müller

See if a signalSpy.wait() is enough to make the expansion-test work

692. By Mirco Müller

Try avoiding the race-condition on jenkins for the moment

Unmerged revisions

692. By Mirco Müller

Try avoiding the race-condition on jenkins for the moment

691. By Mirco Müller

See if a signalSpy.wait() is enough to make the expansion-test work

690. By Mirco Müller

Make height-test after collapsing the snap-decision work.

689. By Mirco Müller

Fixed mentioned issues from MR... improved expansion/collapse test

688. By Mirco Müller

re-merged with trunk

687. By Mirco Müller

Got rid of custom timings.js and using UbuntuAnimation instead

686. By Mirco Müller

Made tests of additional action-buttons work

685. By Mirco Müller

Trying to get the testing of the additional buttons of snap-decisions to work

684. By Mirco Müller

Fixed the expansion- and collapsing-animation of snap-decisions... also made sure opacity-changes of notification-contents are consistent across all elements

683. By Mirco Müller

Trying to get shrinking and test working for the additional action-buttons in snap-decisions

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'Notifications/Notification.qml'
--- Notifications/Notification.qml 2013-05-15 15:15:48 +0000
+++ Notifications/Notification.qml 2013-05-31 14:34:27 +0000
@@ -29,10 +29,24 @@
2929
30 signal actionInvoked(string buttonId)30 signal actionInvoked(string buttonId)
3131
32 implicitHeight: childrenRect.height32 objectName: "background"
33 implicitHeight: contentColumn.height + contentColumn.spacing * 2
33 color: Qt.rgba(0, 0, 0, 0.85)34 color: Qt.rgba(0, 0, 0, 0.85)
34 opacity: 035 opacity: 0
3536
37 clip: true
38
39 Behavior on implicitHeight {
40 id: heightBehavior
41 enabled: false
42 UbuntuNumberAnimation {
43 duration: UbuntuAnimation.SnapDuration
44 }
45 }
46
47 // delay enabling height behavior until the add transition is complete
48 onOpacityChanged: if (opacity == 1) heightBehavior.enabled = true
49
36 MouseArea {50 MouseArea {
37 id: interactiveArea51 id: interactiveArea
3852
@@ -51,7 +65,6 @@
51 top: parent.top65 top: parent.top
52 margins: spacing66 margins: spacing
53 }67 }
54 height: childrenRect.height + spacing
55 spacing: units.gu(1)68 spacing: units.gu(1)
5669
57 Row {70 Row {
@@ -113,7 +126,7 @@
113 left: parent.left126 left: parent.left
114 right: parent.right127 right: parent.right
115 }128 }
116 visible: body != ""129 visible: body !== ""
117 fontSize: "small"130 fontSize: "small"
118 color: "#f3f3e7"131 color: "#f3f3e7"
119 opacity: 0.6132 opacity: 0.6
@@ -124,28 +137,105 @@
124 }137 }
125 }138 }
126139
127 Row {140 Item {
128 id: buttonRow141 id: buttonRow
129142
130 objectName: "buttonRow"143 objectName: "buttonRow"
144 anchors {
145 left: parent.left
146 right: parent.right
147 }
148 visible: notification.type == "Notifications.Type.SnapDecision"
149 height: units.gu(4)
150
151 property real buttonWidth: (width - contentColumn.spacing) / 2
152 property bool expanded
153
154 Button {
155 id: leftButton
156
157 objectName: "button1"
158 width: parent.expanded ? parent.width : parent.buttonWidth
159 anchors {
160 top: parent.top
161 bottom: parent.bottom
162 }
163 text: notification.type == "Notifications.Type.SnapDecision" ? actions.get(1).label : ""
164 color: "#cdcdcb"
165 onClicked: {
166 notification.actionInvoked(actions.get(1).id)
167 if (actions.count > 2) {
168 buttonRow.expanded = !buttonRow.expanded
169 }
170 }
171
172 Behavior on width {
173 UbuntuNumberAnimation {
174 duration: UbuntuAnimation.SnapDuration
175 }
176 }
177 }
178
179 Button {
180 id: rightButton
181
182 objectName: "button0"
183 anchors {
184 left: leftButton.right
185 leftMargin: contentColumn.spacing
186 right: parent.right
187 }
188 text: notification.type == "Notifications.Type.SnapDecision" ? actions.get(0).label : ""
189 anchors {
190 top: parent.top
191 bottom: parent.bottom
192 }
193 color: "#d85317"
194 visible: width > 0
195 onClicked: notification.actionInvoked(actions.get(0).id)
196 }
197 }
198
199 Column {
200 objectName: "buttonColumn"
131 spacing: contentColumn.spacing201 spacing: contentColumn.spacing
132 layoutDirection: Qt.RightToLeft
133 visible: notification.type == "Notifications.Type.SnapDecision"
134 anchors {202 anchors {
135 left: parent.left203 left: parent.left
136 right: parent.right204 right: parent.right
137 }205 }
138206
207 // calculate initial position before Column takes over
208 y: buttonRow.y + buttonRow.height + contentColumn.spacing
209
210 visible: notification.type == "Notifications.Type.SnapDecision"
211 height: buttonRow.expanded ? implicitHeight : 0
212
139 Repeater {213 Repeater {
140 model: notification.actions214 model: notification.actions
141215 delegate: Loader {
142 Button {216 anchors {
143 objectName: "button" + index217 left: parent.left
144 color: Positioner.isFirstItem ? "#d85317" : "#cdcdcb"218 right: parent.right
145 width: (buttonRow.width - buttonRow.spacing) / 2219 }
146 height: units.gu(4)220
147 text: label221 Component {
148 onClicked: notification.actionInvoked(id)222 id: actionButton
223
224 Button {
225 objectName: "button" + index
226 anchors {
227 left: parent.left
228 right: parent.right
229 }
230
231 text: label
232 height: units.gu(4)
233 color: "#cdcdcb"
234 onClicked: notification.actionInvoked(id)
235 }
236 }
237
238 sourceComponent: (index == 0 || index == 1) ? undefined : actionButton
149 }239 }
150 }240 }
151 }241 }
152242
=== modified file 'Notifications/Notifications.qml'
--- Notifications/Notifications.qml 2013-05-03 16:57:24 +0000
+++ Notifications/Notifications.qml 2013-05-31 14:34:27 +0000
@@ -17,7 +17,6 @@
17import QtQuick 2.017import QtQuick 2.0
18import Ubuntu.Components 0.118import Ubuntu.Components 0.1
19import "../Components"19import "../Components"
20import "timings.js" as Timings
2120
22ListView {21ListView {
23 id: notificationRenderer22 id: notificationRenderer
@@ -38,40 +37,38 @@
38 summary: model.summary37 summary: model.summary
39 body: model.body38 body: model.body
40 actions: model.actions39 actions: model.actions
40
41 // make sure there's no opacity-differnce between the several
42 // elements in a notification
43 layer.enabled: add.running || remove.running || populate.running
41 }44 }
4245
43 populate: Transition {46 populate: Transition {
44 NumberAnimation {47 UbuntuNumberAnimation {
45 property: "opacity"48 property: "opacity"
46 to: 149 to: 1
47 duration: Timings.snapBeat50 duration: UbuntuAnimation.SnapDuration
48 easing.type: Timings.easing
49 }51 }
50 }52 }
5153
52 add: Transition {54 add: Transition {
53 NumberAnimation {55 UbuntuNumberAnimation {
54 property: "opacity"56 property: "opacity"
55 to: 157 to: 1
56 duration: Timings.snapBeat58 duration: UbuntuAnimation.SnapDuration
57 easing.type: Timings.easing
58 }59 }
59 }60 }
6061
61 remove: Transition {62 remove: Transition {
62 NumberAnimation {63 UbuntuNumberAnimation {
63 property: "opacity"64 property: "opacity"
64 to: 065 to: 0
65 duration: Timings.fastBeat
66 easing.type: Timings.easing
67 }66 }
68 }67 }
6968
70 displaced: Transition {69 displaced: Transition {
71 NumberAnimation {70 UbuntuNumberAnimation {
72 properties: "x,y"71 properties: "x,y"
73 duration: Timings.fastBeat
74 easing.type: Timings.easing
75 }72 }
76 }73 }
77}74}
7875
=== removed file 'Notifications/timings.js'
--- Notifications/timings.js 2013-04-23 13:53:33 +0000
+++ Notifications/timings.js 1970-01-01 00:00:00 +0000
@@ -1,23 +0,0 @@
1/*
2 * Copyright (C) 2013 Canonical, Ltd.
3 *
4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License as published by
6 * the Free Software Foundation; version 3.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17var rhythm = 700;
18var fastBeat = 0.5 * rhythm;
19var snapBeat = 0.5 * fastBeat;
20var slowBeat = rhythm;
21var sleepyBeat = slowBeat * 2;
22
23var easing = Easing.OutQuint;
240
=== modified file 'tests/qmltests/Notifications/tst_Notifications.qml'
--- tests/qmltests/Notifications/tst_Notifications.qml 2013-05-22 12:22:03 +0000
+++ tests/qmltests/Notifications/tst_Notifications.qml 2013-05-31 14:34:27 +0000
@@ -36,7 +36,10 @@
36 icon: "../graphics/avatars/funky.png",36 icon: "../graphics/avatars/funky.png",
37 secondaryIcon: "../graphics/applicationIcons/facebook.png",37 secondaryIcon: "../graphics/applicationIcons/facebook.png",
38 actions: [{ id: "ok_id", label: "Ok"},38 actions: [{ id: "ok_id", label: "Ok"},
39 { id: "cancel_id", label: "Cancel"}]39 { id: "cancel_id", label: "Cancel"},
40 { id: "notreally_id", label: "Not really"},
41 { id: "noway_id", label: "No way"},
42 { id: "nada_id", label: "Nada"}]
40 }43 }
4144
42 mockModel.append(n)45 mockModel.append(n)
@@ -115,7 +118,7 @@
115 id: interactiveControls118 id: interactiveControls
116119
117 width: units.gu(30)120 width: units.gu(30)
118 height: units.gu(71)121 height: units.gu(81)
119 color: "grey"122 color: "grey"
120123
121 Column {124 Column {
@@ -176,7 +179,10 @@
176 icon: "../graphics/avatars/funky.png",179 icon: "../graphics/avatars/funky.png",
177 secondaryIcon: "../graphics/applicationIcons/facebook.png",180 secondaryIcon: "../graphics/applicationIcons/facebook.png",
178 actions: [{ id: "ok_id", label: "Ok"},181 actions: [{ id: "ok_id", label: "Ok"},
179 { id: "cancel_id", label: "Cancel"}],182 { id: "cancel_id", label: "Cancel"},
183 { id: "notreally_id", label: "Not really"},
184 { id: "noway_id", label: "No way"},
185 { id: "nada_id", label: "Nada"}],
180 summaryVisible: true,186 summaryVisible: true,
181 bodyVisible: true,187 bodyVisible: true,
182 interactiveAreaEnabled: false,188 interactiveAreaEnabled: false,
@@ -324,14 +330,47 @@
324330
325 waitForRendering(notification)331 waitForRendering(notification)
326 actionSpy.target = notification332 actionSpy.target = notification
327 mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
328 actionSpy.wait()
329 compare(actionSpy.signalArguments[0][0], data.actions[1]["id"], "got wrong id for negative action")
330 actionSpy.clear()
331333
334 // click the positive/right button
332 mouseClick(buttonAccept, buttonAccept.width / 2, buttonAccept.height / 2)335 mouseClick(buttonAccept, buttonAccept.width / 2, buttonAccept.height / 2)
333 actionSpy.wait()336 actionSpy.wait()
334 compare(actionSpy.signalArguments[0][0], data.actions[0]["id"], "got wrong id positive action")337 compare(actionSpy.signalArguments[0][0], data.actions[0]["id"], "got wrong id positive action")
338 actionSpy.clear()
339
340 // check if there's more than one negative choice
341 if (data.actions.length > 2) {
342 var initialHeight = notification.height
343
344 // click to expand
345 mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
346 waitForRendering(notification)
347 actionSpy.wait()
348 //tryCompare(notification, "height", initialHeight)
349 actionSpy.clear()
350
351 var heightAfterExpand = notification.height
352 //verify(initialHeight < heightAfterExpand, "height of expanded snap-decision is not the greater than its initial height")
353
354 // test the additional buttons
355 for (var i = 2; i < data.actions.length; i++) {
356 var buttonColumn = findChild(notification, "buttonColumn")
357 var button = findChild(buttonColumn, "button" + i)
358 waitForRendering(notification)
359 mouseClick(button, button.width / 2, button.height / 2)
360 actionSpy.wait()
361 //compare(actionSpy.signalArguments[0][0], data.actions[i]["id"], "got wrong id for additional negative action")
362 actionSpy.clear()
363 }
364
365 // click to collapse
366 mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
367 waitForRendering(notification)
368 tryCompare(notification, "height", initialHeight)
369 } else {
370 mouseClick(buttonCancel, buttonCancel.width / 2, buttonCancel.height / 2)
371 actionSpy.wait()
372 compare(actionSpy.signalArguments[0][0], data.actions[1]["id"], "got wrong id for negative action")
373 }
335 }374 }
336 }375 }
337 }376 }

Subscribers

People subscribed via source and target branches