Merge lp:~zsombi/ubuntu-ui-toolkit/aplRemovePages into lp:ubuntu-ui-toolkit/staging
- aplRemovePages
- Merge into staging
Status: | Merged |
---|---|
Approved by: | Tim Peeters |
Approved revision: | 1888 |
Merged at revision: | 1898 |
Proposed branch: | lp:~zsombi/ubuntu-ui-toolkit/aplRemovePages |
Merge into: | lp:ubuntu-ui-toolkit/staging |
Diff against target: |
259 lines (+107/-29) 3 files modified
src/Ubuntu/Components/1.3/AdaptivePageLayout.qml (+5/-4) tests/unit_x11/tst_components/MyExternalPageWithNewHeader.qml (+26/-0) tests/unit_x11/tst_components/tst_adaptivepagelayout.qml (+76/-25) |
To merge this branch: | bzr merge lp:~zsombi/ubuntu-ui-toolkit/aplRemovePages |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
ubuntu-sdk-build-bot | continuous-integration | Approve | |
Tim Peeters | Approve | ||
Review via email: mp+288090@code.launchpad.net |
Commit message
AdaptivePageLayout removes all pages pushed to next columns when addPageToNextCo
Description of the change
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1884
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1884
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1884
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1884
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
- 1885. By Zsombor Egri
-
roll back changes on MyExternalPage and add a new one to be used by the APL tests
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1885
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1885
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1885
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1885
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1885
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
FAILURE: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Tim Peeters (tpeeters) wrote : | # |
Only one small comment (see below).
Also, the tst_tree tests fail. That is unrelated to this MR but those need to be fixed before we can land this.
Tim Peeters (tpeeters) : | # |
- 1886. By Zsombor Egri
-
review comment applied
- 1887. By Zsombor Egri
-
staging sync
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1887
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1887
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1887
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1887
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1887
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Tim Peeters (tpeeters) wrote : | # |
There is one failure in tst_adaptivepag
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1887
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
Zsombor Egri (zsombi) wrote : | # |
> There is one failure in tst_adaptivepag
> tst_adaptivepag
> components:
> heightChanged' returned TRUE unexpectedly. ()
Dear Tim, that was a test you were supposed to look at it >10 days ago...
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
FAILED: Continuous integration, rev:1887
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
- 1888. By Zsombor Egri
-
staging sync
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1887
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1888
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1888
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1888
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1888
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1888
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
SUCCESS: https:/
None: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
Click here to trigger a rebuild:
https:/
Tim Peeters (tpeeters) wrote : | # |
All the dependencies are in place now :) Let's get this one in too.
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1888
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1888
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1888
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote : | # |
PASSED: Continuous integration, rev:1888
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild:
https:/
Preview Diff
1 | === modified file 'src/Ubuntu/Components/1.3/AdaptivePageLayout.qml' |
2 | --- src/Ubuntu/Components/1.3/AdaptivePageLayout.qml 2016-03-15 14:39:45 +0000 |
3 | +++ src/Ubuntu/Components/1.3/AdaptivePageLayout.qml 2016-03-16 09:15:53 +0000 |
4 | @@ -339,11 +339,14 @@ |
5 | */ |
6 | function addPageToNextColumn(sourcePage, page, properties) { |
7 | var nextColumn = d.columnForPage(sourcePage) + 1; |
8 | - d.tree.prune(nextColumn); |
9 | + var wrappers = d.tree.prune(nextColumn); |
10 | return d.addPageToColumn(nextColumn, sourcePage, page, properties, function() { |
11 | for (var i = nextColumn; i < d.columns; i++) { |
12 | d.updatePageForColumn(i); |
13 | } |
14 | + for (var i in wrappers) { |
15 | + wrappers[i].destroyObject(); |
16 | + } |
17 | }); |
18 | } |
19 | |
20 | @@ -591,9 +594,7 @@ |
21 | if (newWrapper) { |
22 | columnHolder.attachPage(newWrapper); |
23 | } |
24 | - if (oldWrapper.canDestroy) { |
25 | - oldWrapper.destroyObject(); |
26 | - } |
27 | + oldWrapper.destroyObject(); |
28 | } |
29 | } |
30 | |
31 | |
32 | === added file 'tests/unit_x11/tst_components/MyExternalPageWithNewHeader.qml' |
33 | --- tests/unit_x11/tst_components/MyExternalPageWithNewHeader.qml 1970-01-01 00:00:00 +0000 |
34 | +++ tests/unit_x11/tst_components/MyExternalPageWithNewHeader.qml 2016-03-16 09:15:53 +0000 |
35 | @@ -0,0 +1,26 @@ |
36 | +/* |
37 | + * Copyright 2016 Canonical Ltd. |
38 | + * |
39 | + * This program is free software; you can redistribute it and/or modify |
40 | + * it under the terms of the GNU Lesser General Public License as published by |
41 | + * the Free Software Foundation; version 3. |
42 | + * |
43 | + * This program is distributed in the hope that it will be useful, |
44 | + * but WITHOUT ANY WARRANTY; without even the implied warranty of |
45 | + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the |
46 | + * GNU Lesser General Public License for more details. |
47 | + * |
48 | + * You should have received a copy of the GNU Lesser General Public License |
49 | + * along with this program. If not, see <http://www.gnu.org/licenses/>. |
50 | + */ |
51 | + |
52 | +import QtQuick 2.4 |
53 | +import Ubuntu.Components 1.3 |
54 | + |
55 | +Page { |
56 | + header: PageHeader { title: "Page from QML file" } |
57 | + Label { |
58 | + anchors.centerIn: parent |
59 | + text: "This page was created from MyExternalPageWithNewHeader.qml." |
60 | + } |
61 | +} |
62 | |
63 | === modified file 'tests/unit_x11/tst_components/tst_adaptivepagelayout.qml' |
64 | --- tests/unit_x11/tst_components/tst_adaptivepagelayout.qml 2016-02-10 14:03:36 +0000 |
65 | +++ tests/unit_x11/tst_components/tst_adaptivepagelayout.qml 2016-03-16 09:15:53 +0000 |
66 | @@ -38,8 +38,8 @@ |
67 | |
68 | Page { |
69 | id: page1 |
70 | - objectName: title |
71 | - title: "Page1" |
72 | + objectName: header.title |
73 | + header: PageHeader { title: "Page1" } |
74 | |
75 | Column { |
76 | anchors.centerIn: parent |
77 | @@ -56,18 +56,18 @@ |
78 | } |
79 | Page { |
80 | id: page2 |
81 | - objectName: title |
82 | - title: "Page2" |
83 | + objectName: header.title |
84 | + header: PageHeader { title: "Page2" } |
85 | } |
86 | Page { |
87 | id: page3 |
88 | - objectName: title |
89 | - title: "Page3" |
90 | + objectName: header.title |
91 | + header: PageHeader { title: "Page3" } |
92 | } |
93 | Page { |
94 | id: page4 |
95 | - objectName: title |
96 | - title: "Page4" |
97 | + objectName: header.title |
98 | + header: PageHeader { title: "Page4" } |
99 | } |
100 | } |
101 | AdaptivePageLayout { |
102 | @@ -76,18 +76,18 @@ |
103 | height: parent.height / 2 |
104 | Page { |
105 | id: otherPage1 |
106 | - objectName: title |
107 | - title: "Page1" |
108 | + objectName: header.title |
109 | + header: PageHeader { title: "Page1" } |
110 | } |
111 | Page { |
112 | id: otherPage2 |
113 | - objectName: title |
114 | - title: "Page2" |
115 | + objectName: header.title |
116 | + header: PageHeader { title: "Page2" } |
117 | } |
118 | Page { |
119 | id: otherPage3 |
120 | - objectName: title |
121 | - title: "Page3" |
122 | + objectName: header.title |
123 | + header: PageHeader { title: "Page3" } |
124 | } |
125 | } |
126 | } |
127 | @@ -95,8 +95,10 @@ |
128 | Component { |
129 | id: pageComponent |
130 | Page { |
131 | - objectName: title |
132 | - title: "DynamicPage" |
133 | + objectName: header.title |
134 | + header: PageHeader { title: "DynamicPage" } |
135 | + signal deleted() |
136 | + Component.onDestruction: deleted() |
137 | } |
138 | } |
139 | |
140 | @@ -116,6 +118,16 @@ |
141 | signalName: "primaryPageChanged" |
142 | } |
143 | |
144 | + SignalSpy { |
145 | + id: deletedSpy1 |
146 | + signalName: "deleted" |
147 | + } |
148 | + |
149 | + SignalSpy { |
150 | + id: deletedSpy2 |
151 | + signalName: "deleted" |
152 | + } |
153 | + |
154 | function resize_single_column() { |
155 | layout.width = units.gu(40); |
156 | } |
157 | @@ -142,16 +154,20 @@ |
158 | } |
159 | |
160 | function cleanup() { |
161 | - page1.title = "Page1"; |
162 | - page2.title = "Page2"; |
163 | - page3.title = "Page3"; |
164 | - page4.title = "Page4"; |
165 | + page1.header.title = "Page1"; |
166 | + page2.header.title = "Page2"; |
167 | + page3.header.title = "Page3"; |
168 | + page4.header.title = "Page4"; |
169 | loadedSpy.clear(); |
170 | primaryPageSpy.clear(); |
171 | primaryPageSpy.target = null; |
172 | resize_multiple_columns(); |
173 | layout.removePages(layout.primaryPage); |
174 | defaults.primaryPage = null; |
175 | + // restore binding on column number |
176 | + root.columns = Qt.binding(function () {return root.width >= units.gu(80) ? 2 : 1}); |
177 | + // restore async |
178 | + layout.asynchronous = true; |
179 | wait(200); |
180 | } |
181 | |
182 | @@ -306,7 +322,7 @@ |
183 | var testColumn = MathUtils.clamp(wrapper.column + (data.func == "addPageToCurrentColumn" ? 0 : 1), |
184 | 0, layout.columns - 1); |
185 | var testHolder = getColumnHolder(layout, testColumn); |
186 | - compare(testHolder.pageWrapper.object.title, data.expectedTitle, "page not found"); |
187 | + compare(testHolder.pageWrapper.object.header.title, data.expectedTitle, "page not found"); |
188 | } |
189 | |
190 | function test_asynchronous_page_loading_incubator_forcecompletion() { |
191 | @@ -383,7 +399,7 @@ |
192 | function test_primaryPageSource_bug1499179_data() { |
193 | return [ |
194 | {tag: "Component", test: pageComponent}, |
195 | - {tag: "Document", test: Qt.resolvedUrl("MyExternalPage.qml")}, |
196 | + {tag: "Document", test: Qt.resolvedUrl("MyExternalPageWithNewHeader.qml")}, |
197 | ]; |
198 | } |
199 | function test_primaryPageSource_bug1499179(data) { |
200 | @@ -394,8 +410,8 @@ |
201 | |
202 | function test_change_primaryPageSource_data() { |
203 | return [ |
204 | - {tag: "Component", test: pageComponent, nextValue: Qt.resolvedUrl("MyExternalPage.qml")}, |
205 | - {tag: "Document", test: Qt.resolvedUrl("MyExternalPage.qml"), nextValue: pageComponent}, |
206 | + {tag: "Component", test: pageComponent, nextValue: Qt.resolvedUrl("MyExternalPageWithNewHeader.qml")}, |
207 | + {tag: "Document", test: Qt.resolvedUrl("MyExternalPageWithNewHeader.qml"), nextValue: pageComponent}, |
208 | ]; |
209 | } |
210 | function test_change_primaryPageSource(data) { |
211 | @@ -428,12 +444,47 @@ |
212 | primaryPageSpy.target = defaults; |
213 | defaults.primaryPageSource = pageComponent; |
214 | primaryPageSpy.wait(400); |
215 | - compare(defaults.primaryPage.title, "DynamicPage", "DynamicPage not set as primaryPage"); |
216 | + compare(defaults.primaryPage.header.title, "DynamicPage", "DynamicPage not set as primaryPage"); |
217 | // now set a value to primaryPage |
218 | primaryPageSpy.clear(); |
219 | defaults.primaryPage = otherPage1; |
220 | primaryPageSpy.wait(400); |
221 | compare(defaults.primaryPageSource, undefined, "primaryPageSource must be cleared"); |
222 | } |
223 | + |
224 | + function test_add_page_to_next_column_doesnt_delete_prev_column_content_bug1544745() { |
225 | + |
226 | + var incubator = layout.addPageToNextColumn(layout.primaryPage, pageComponent); |
227 | + verify(incubator); |
228 | + verify(incubator.status != Component.Error); |
229 | + var newPage = null; |
230 | + if (incubator.status == Component.Loading) { |
231 | + incubator.forceCompletion(); |
232 | + newPage = incubator.object; |
233 | + } else if (incubator.status == Component.Ready) { |
234 | + newPage = incubator.object; |
235 | + } |
236 | + deletedSpy1.target = newPage; |
237 | + |
238 | + // add one page on top of the page on second column |
239 | + incubator = layout.addPageToCurrentColumn(newPage, pageComponent); |
240 | + verify(incubator); |
241 | + newPage = null; |
242 | + if (incubator.status == Component.Loading) { |
243 | + incubator.forceCompletion(); |
244 | + newPage = incubator.object; |
245 | + } else if (incubator.status == Component.Ready) { |
246 | + newPage = incubator.object; |
247 | + } |
248 | + deletedSpy2.target = newPage; |
249 | + |
250 | + // add a new page relative to the primary page to the next column |
251 | + // this should remove the previously added two pages |
252 | + incubator = layout.addPageToNextColumn(layout.primaryPage, pageComponent); |
253 | + verify(incubator); |
254 | + incubator.forceCompletion(); |
255 | + deletedSpy1.wait(500); |
256 | + deletedSpy2.wait(500); |
257 | + } |
258 | } |
259 | } |
FAILED: Continuous integration, rev:1884 /jenkins. ubuntu. com/ubuntu- sdk/job/ ubuntu- ui-toolkit- ci-amd64- stable/ 439/ /jenkins. ubuntu. com/ubuntu- sdk/job/ generic- update- mp/1495/ console
https:/
Executed test runs:
None: https:/
Click here to trigger a rebuild: /jenkins. ubuntu. com/ubuntu- sdk/job/ ubuntu- ui-toolkit- ci-amd64- stable/ 439/rebuild
https:/