Unity 8

Merge lp:~cimi/unity8/overlay-right-padding into lp:unity8

  1. overlay-right-padding
  2. Merge into trunk
Proposed by Andrea Cimitan
Status: Merged
Approved by: Michael Zanetti
Approved revision: 1171
Merged at revision: 1209
Proposed branch: lp:~cimi/unity8/overlay-right-padding
Merge into: lp:unity8
Prerequisite: lp:~saviq/unity8/fix-empty-attributes
Diff against target: 210 lines (+66/-1)
3 files modified
plugins/Dash/CardCreator.js (+2/-0)
tests/plugins/Dash/cardcreator/5.res (+2/-0)
tests/qmltests/Dash/tst_Card.qml (+62/-1)
To merge this branch: bzr merge lp:~cimi/unity8/overlay-right-padding
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Michael Zanetti (community) Approve
Review via email: mp+231586@code.launchpad.net

Commit message

Fix right padding on overlay card

Description of the change

 * Are there any related MPs required for this MP to build/function as expected? Please list.
No
 * 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

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

PASSED: Continuous integration, rev:1167
http://jenkins.qa.ubuntu.com/job/unity8-ci/4040/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3849
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1041
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1134
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1134
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3707
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5096
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5096/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11826

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4040/rebuild

review: Approve (continuous-integration)
lp:~cimi/unity8/overlay-right-padding updated
1167. By Andrea Cimitan

Add test for left/right paddings

1168. By Andrea Cimitan

Fix right padding and tests

1169. By Andrea Cimitan

Remove prefix for padding test

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

FAILED: Continuous integration, rev:1169
http://jenkins.qa.ubuntu.com/job/unity8-ci/4052/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3900/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1053/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1146/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1146/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5147/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4052/rebuild

review: Needs Fixing (continuous-integration)
lp:~cimi/unity8/overlay-right-padding updated
1170. By Andrea Cimitan

Whitespaces

Revision history for this message
Michael Zanetti (mzanetti) wrote :

QWARN : qmltestrunner::Card::test_paddings(No Summary) file:///home/mzanetti/Development/reviews/overlay-right-padding/builddir/plugins/Dash/CardCreator.js:660: Error: Qt.createQmlObject(): failed to create object:
    file:///home/mzanetti/Development/reviews/overlay-right-padding/builddir/plugins/Dash/createCardComponent:31:17: Expected token `;'
FAIL! : qmltestrunner::Card::test_paddings(No Summary) 'verify()' returned FALSE. ()
   Loc: [/home/mzanetti/Development/reviews/overlay-right-padding/tests/qmltests/Dash/tst_Card.qml(536)]

review: Needs Fixing
Revision history for this message
Andrea Cimitan (cimi) wrote :

> QWARN : qmltestrunner::Card::test_paddings(No Summary)
> file:///home/mzanetti/Development/reviews/overlay-right-
> padding/builddir/plugins/Dash/CardCreator.js:660: Error: Qt.createQmlObject():
> failed to create object:
> file:///home/mzanetti/Development/reviews/overlay-right-
> padding/builddir/plugins/Dash/createCardComponent:31:17: Expected token `;'
> FAIL! : qmltestrunner::Card::test_paddings(No Summary) 'verify()' returned
> FALSE. ()
> Loc: [/home/mzanetti/Development/reviews/overlay-right-
> padding/tests/qmltests/Dash/tst_Card.qml(536)]

Which version did you try? works for me... but I still have this createCardComponent:31:17: Expected token `; that is also everywhere else and does not compromise look

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

FAILED: Continuous integration, rev:1170
http://jenkins.qa.ubuntu.com/job/unity8-ci/4058/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3917
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1059
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1152
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1152
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3770
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5164
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5164/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11914

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4058/rebuild

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

> > QWARN : qmltestrunner::Card::test_paddings(No Summary)
> > file:///home/mzanetti/Development/reviews/overlay-right-
> > padding/builddir/plugins/Dash/CardCreator.js:660: Error:
> Qt.createQmlObject():
> > failed to create object:
> > file:///home/mzanetti/Development/reviews/overlay-right-
> > padding/builddir/plugins/Dash/createCardComponent:31:17: Expected token `;'
> > FAIL! : qmltestrunner::Card::test_paddings(No Summary) 'verify()' returned
> > FALSE. ()
> > Loc: [/home/mzanetti/Development/reviews/overlay-right-
> > padding/tests/qmltests/Dash/tst_Card.qml(536)]
>
> Which version did you try? works for me... but I still have this
> createCardComponent:31:17: Expected token `; that is also everywhere else and
> does not compromise look

figured this only happens if GRID_UNIT_PX isn't 8 (in my case 16)

lp:~cimi/unity8/overlay-right-padding updated
1171. By Andrea Cimitan

More stable tests

Revision history for this message
Michael Zanetti (mzanetti) wrote :

tests working here now. Change looks good and afaict doesn't break other layouts

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

yes

 * Did CI run pass? If not, please explain why.

waiting on it

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

PASSED: Continuous integration, rev:1171
http://jenkins.qa.ubuntu.com/job/unity8-ci/4070/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/3958
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-utopic/1071
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/1164
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/1164
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/3807
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5205
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/5205/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/11967

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/4070/rebuild

review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/Dash/CardCreator.js'
2--- plugins/Dash/CardCreator.js 2014-08-22 14:43:16 +0000
3+++ plugins/Dash/CardCreator.js 2014-08-22 14:43:17 +0000
4@@ -467,6 +467,8 @@
5 if (!touchdownOnArtShape) {
6 extraRightAnchor = 'rightMargin: units.gu(1); \n';
7 extraLeftAnchor = 'leftMargin: units.gu(1); \n';
8+ } else if (headerAsOverlay && !hasEmblem) {
9+ extraRightAnchor = 'rightMargin: units.gu(1); \n';
10 }
11
12 if (hasMascot) {
13
14=== modified file 'tests/plugins/Dash/cardcreator/5.res'
15--- tests/plugins/Dash/cardcreator/5.res 2014-08-22 14:43:16 +0000
16+++ tests/plugins/Dash/cardcreator/5.res 2014-08-22 14:43:17 +0000
17@@ -103,6 +103,7 @@
18 id: titleLabel;
19 objectName: "titleLabel";
20 anchors { right: parent.right;
21+ rightMargin: units.gu(1);
22 left: parent.left;
23 leftMargin: units.gu(1);
24 top: overlayLoader.top;
25@@ -124,6 +125,7 @@
26 objectName: "subtitleLabel";
27 anchors { left: titleLabel.left;
28 leftMargin: titleLabel.leftMargin;
29+ rightMargin: units.gu(1);
30 right: titleLabel.right;
31 top: titleLabel.bottom;
32 }
33
34=== modified file 'tests/qmltests/Dash/tst_Card.qml'
35--- tests/qmltests/Dash/tst_Card.qml 2014-08-13 10:29:21 +0000
36+++ tests/qmltests/Dash/tst_Card.qml 2014-08-22 14:43:17 +0000
37@@ -96,6 +96,11 @@
38 "layout": { "template": { "card-background": Qt.resolvedUrl("artwork/checkers.png") },
39 "components": JSON.parse(Helpers.fullMapping) }
40 },
41+ {
42+ "name": "Art, title - overlaid",
43+ "layout": { "template": { "overlay": true },
44+ "components": { "art": "art", "title": "title" } }
45+ },
46 ]
47
48 CardTool {
49@@ -217,6 +222,7 @@
50
51 function test_card_binding(data) {
52 selector.selectedIndex = data.index;
53+ waitForRendering(selector);
54 waitForRendering(card);
55
56 tryCompareFunction(function() { return testCase[data.object] !== null }, true);
57@@ -247,6 +253,8 @@
58
59 function test_card_size(data) {
60 selector.selectedIndex = data.index;
61+ waitForRendering(selector);
62+ waitForRendering(card);
63
64 if (data.hasOwnProperty("card_layout")) {
65 cardTool.template['card-layout'] = data.card_layout;
66@@ -279,6 +287,7 @@
67
68 function test_art_size(data) {
69 selector.selectedIndex = data.index;
70+ waitForRendering(selector);
71 if (data.hasOwnProperty("size")) {
72 cardTool.template['card-size'] = data.size;
73 cardTool.templateChanged();
74@@ -310,6 +319,7 @@
75
76 function test_art_shape_fixed_size() {
77 selector.selectedIndex = 6;
78+ waitForRendering(selector);
79 card.fixedArtShapeSize = Qt.size( units.gu(8), units.gu(4) );
80 waitForRendering(card);
81 tryCompare(art, "width", units.gu(8));
82@@ -326,6 +336,7 @@
83
84 function test_art_layout(data) {
85 selector.selectedIndex = data.index;
86+ waitForRendering(selector);
87 waitForRendering(card);
88
89 tryCompare(headerRow, "x", data.left());
90@@ -344,6 +355,7 @@
91
92 function test_header_layout(data) {
93 selector.selectedIndex = data.index;
94+ waitForRendering(selector);
95 waitForRendering(card);
96
97 tryCompareFunction(function() { return testCase.headerRow.y === data.top() }, true);
98@@ -359,7 +371,7 @@
99
100 function test_summary_layout(data) {
101 selector.selectedIndex = data.index;
102-
103+ waitForRendering(selector);
104 waitForRendering(card);
105
106 tryCompareFunction(function() { return art.height > 0 && testCase.summary.y === data.top() }, true);
107@@ -367,6 +379,8 @@
108
109 function test_art_visibility() {
110 selector.selectedIndex = 8;
111+ waitForRendering(selector);
112+ waitForRendering(card);
113
114 compare(testCase.artImage, null);
115 compare(testCase.art, null);
116@@ -389,6 +403,7 @@
117
118 function test_background(data) {
119 selector.selectedIndex = data.index;
120+ waitForRendering(selector);
121
122 if (data.hasOwnProperty("background")) {
123 card.cardData["background"] = data.background;
124@@ -425,6 +440,8 @@
125
126 function test_font_weights(data) {
127 selector.selectedIndex = data.index;
128+ waitForRendering(selector);
129+ waitForRendering(card);
130
131 tryCompare(testCase.title.font, "weight", data.weight);
132 }
133@@ -448,6 +465,7 @@
134
135 function test_fontColor(data) {
136 selector.selectedIndex = 10;
137+ waitForRendering(selector);
138 waitForRendering(card);
139
140 background.color = data.tag;
141@@ -470,6 +488,7 @@
142
143 function test_emblemIcon(data) {
144 selector.selectedIndex = data.index;
145+ waitForRendering(selector);
146 waitForRendering(card);
147
148 var emblemIcon = findChild(card, "emblemIcon");
149@@ -486,6 +505,7 @@
150
151 function test_mascotShape(data) {
152 selector.selectedIndex = data.index;
153+ waitForRendering(selector);
154 waitForRendering(card);
155
156 var shape = findChild(card, "mascotShapeLoader");
157@@ -496,6 +516,8 @@
158
159 function test_touchdown_visibility() {
160 selector.selectedIndex = 0;
161+ waitForRendering(selector);
162+ waitForRendering(card);
163
164 var touchdown = findChild(card, "touchdown");
165
166@@ -505,5 +527,44 @@
167 mouseRelease(card, card.width/2, card.height/2);
168 compare(touchdown.visible, false);
169 }
170+
171+ function test_paddings_data() {
172+ return [
173+ { tag: "Art and summary", index: 0 },
174+ { tag: "No Summary", index: 6 },
175+ { tag: "No header", index: 7 },
176+ { tag: "Header only", index: 8 },
177+ { tag: "Art, header, summary - overlaid", index: 9 },
178+ { tag: "Art, title - overlaid", index: 13 },
179+ ];
180+ }
181+
182+ function test_paddings(data) {
183+ selector.selectedIndex = data.index;
184+ waitForRendering(selector);
185+ waitForRendering(card);
186+
187+ if (title) var titleToCard = title.mapToItem(card, 0, 0, title.width, title.height);
188+
189+ // left margin
190+ if (mascotImage) {
191+ var mascotToCard = mascotImage.mapToItem(card, 0, 0, mascotImage.width, mascotImage.height);
192+ verify(mascotToCard.x === units.gu(1));
193+ if (title) {
194+ verify((titleToCard.x - mascotToCard.x - mascotToCard.width) === units.gu(1));
195+ }
196+ } else if (title) {
197+ verify(titleToCard.x === units.gu(1));
198+ }
199+
200+ // right margin
201+ var emblemIcon = findChild(card, "emblemIcon");
202+ if (emblemIcon) {
203+ var emblemToCard = emblemIcon.mapToItem(card, 0, 0, emblemIcon.width, emblemIcon.height);
204+ verify((card.width - emblemToCard.x - emblemToCard.width) === units.gu(1));
205+ } else if (title) {
206+ verify((card.width - titleToCard.x - titleToCard.width) === units.gu(1));
207+ }
208+ }
209 }
210 }

Subscribers

People subscribed via source and target branches