Merge into trunk : previewIconActionsModelChange : Code : Unity 8

Status:	Merged
Approved by:	Andrea Cimitan on 2016-04-04
Approved revision:	2323
Merged at revision:	2386
Proposed branch:	lp:~aacid/unity8/previewIconActionsModelChange
Merge into:	lp:unity8
Prerequisite:	lp:~aacid/unity8/previewActionsModelChange
Diff against target:	92 lines (+29/-4) 2 files modified qml/Dash/Previews/PreviewIconActions.qml (+15/-0) tests/qmltests/Dash/Previews/tst_PreviewIconActions.qml (+14/-4)
To merge this branch:	bzr merge lp:~aacid/unity8/previewIconActionsModelChange
Related bugs:	Link a bug report

Reviewer	Review Type	Date Requested	Status
Andrea Cimitan (community)		2016-04-01	Approve on 2016-04-04
Unity8 CI Bot	continuous-integration		Needs Fixing on 2016-04-04
Review via email: mp+290748@code.launchpad.net

Commit Message

Update the icon also to the original one if that's what the scope wants

Description of the Change

* Are there any related MPs required for this MP to build/function as expected?
Prereq

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* Did you make sure that your branch does not contain spurious tags?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-01:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/917/rebuild

review: Needs Fixing (continuous-integration)

lp:~aacid/unity8/previewIconActionsModelChange updated on 2016-04-04

2323. By Albert Astals Cid on 2016-04-04: add waitForRenderin

Unity8 CI Bot (unity8-ci-bot) wrote on 2016-04-04:

#

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/923/rebuild

review: Needs Fixing (continuous-integration)

Andrea Cimitan (cimi) on 2016-04-04:

#

review: Needs Fixing

Albert Astals Cid (aacid) wrote on 2016-04-04:

#

> we should just compare the url instead converting to string, like you suggested me here
> https://code.launchpad.net/~cimi/unity8/preview-sharing-string-and-array/+merge/287008/comments/732516

It is hard to do that since the url is relative to the folder you are building the code, it's a different scenario to what i suggested there since for you the urls where set hardcoded on the test.

Andrea Cimitan (cimi) wrote on 2016-04-04:

#

> > we should just compare the url instead converting to string, like you
> suggested me here
> > https://code.launchpad.net/~cimi/unity8/preview-sharing-string-and-
> array/+merge/287008/comments/732516
>
> It is hard to do that since the url is relative to the folder you are building
> the code, it's a different scenario to what i suggested there since for you
> the urls where set hardcoded on the test.

fair enough

* Did you perform an exploratory manual test run of the code change and any related functionality?
y
* Did CI run pass? If not, please explain why.
* Did you make sure that the branch does not contain spurious tags?
y

review: Approve

1	=== modified file 'qml/Dash/Previews/PreviewIconActions.qml'
2	--- qml/Dash/Previews/PreviewIconActions.qml 2015-08-03 13:48:14 +0000
3	+++ qml/Dash/Previews/PreviewIconActions.qml 2016-04-04 11:00:13 +0000
4	@@ -32,10 +32,20 @@
5	id: row
6	readonly property var actions: root.widgetData ? root.widgetData["actions"] : null
7	width: parent.width
8	+ onActionsChanged: {
9	+ // Rewire the source since it may have been unwired on onClicked
10	+ // but the icon didn't change i.e. the scope updated the results
11	+ // and the icon needs to go back to the one it was originally
12	+ for (var i = 0; i < repeater.count; ++i) {
13	+ var button = repeater.itemAt(i);
14	+ button.updateIcon();
15	+ }
16	+ }
17
18	spacing: units.gu(2)
19
20	Repeater {
21	+ id: repeater
22	model: row.actions
23
24	AbstractButton {
25	@@ -43,6 +53,11 @@
26	height: label.height
27	width: childrenRect.width
28
29	+ readonly property var iconUrl: modelData.icon;
30	+ function updateIcon() {
31	+ icon.source = iconUrl;
32	+ }
33	+
34	Image {
35	id: icon
36	height: parent.height
37
38	=== modified file 'tests/qmltests/Dash/Previews/tst_PreviewIconActions.qml'
39	--- tests/qmltests/Dash/Previews/tst_PreviewIconActions.qml 2015-10-26 08:53:52 +0000
40	+++ tests/qmltests/Dash/Previews/tst_PreviewIconActions.qml 2016-04-04 11:00:13 +0000
41	@@ -69,9 +69,12 @@
42	Timer {
43	id: timer
44	property int index: -1
45	+ property bool changeIcon: true
46	interval: 500
47	onTriggered: {
48	- actionDataActions.actions[index].icon = Qt.resolvedUrl("../../UnityLogo.png");
49	+ if (changeIcon) {
50	+ actionDataActions.actions[index].icon = Qt.resolvedUrl("../../UnityLogo.png");
51	+ }
52	preview.widgetDataChanged();
53	index = -1;
54	}
55	@@ -89,12 +92,14 @@
56
57	function test_checkButtonWithTemporary_data() {
58	return [
59	- {tag: "with temporary", temporaryIcon: "emblem", index: 0},
60	- {tag: "without temporary", temporaryIcon: undefined, index: 2},
61	+ {tag: "with temporary change icon", temporaryIcon: "emblem", index: 0, changeIcon: true},
62	+ {tag: "with temporary no change icon", temporaryIcon: "emblem", index: 1, changeIcon: false},
63	+ {tag: "without temporary", temporaryIcon: undefined, index: 2, changeIcon: true},
64	];
65	}
66
67	function test_checkButtonWithTemporary(data) {
68	+ waitForRendering(preview);
69	spy.target = preview;
70
71	var buttonId = actionDataActions.actions[data.index].id;
72	@@ -108,6 +113,7 @@
73
74	compare(image.source, buttonIcon);
75	compare(label.text, buttonLabel);
76	+ timer.changeIcon = data.changeIcon;
77	mouseClick(button);
78	compare(spy.count, 1);
79	compare(spy.signalArguments[0][1], buttonId);
80	@@ -119,7 +125,11 @@
81	tryCompareFunction(function() {
82	var button = findChild(root, "button" + buttonId);
83	var image = findChildsByType(button, "QQuickImage")[0];
84	- return image.source && image.source.toString().indexOf("UnityLogo") > -1;
85	+ if (data.changeIcon) {
86	+ return image.source && image.source.toString().indexOf("UnityLogo") > -1;
87	+ } else {
88	+ return image.source && image.source == buttonIcon;
89	+ }
90	}, true);
91	}
92	}

Unity 8

Merge lp:~aacid/unity8/previewIconActionsModelChange into lp:unity8

Commit Message

Description of the Change

Preview Diff

Subscribers