Ubuntu UI Toolkit

Merge lp:~tpeeters/ubuntu-ui-toolkit/refreshMargin into lp:ubuntu-ui-toolkit/staging

  1. refreshMargin
  2. Merge into staging
Proposed by Tim Peeters on 2016-05-11
Status: Merged
Approved by: Zsombor Egri on 2016-05-13
Approved revision: 1991
Merged at revision: 1977
Proposed branch: lp:~tpeeters/ubuntu-ui-toolkit/refreshMargin
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 293 lines (+182/-18)
4 files modified
src/Ubuntu/Components/Themes/Ambiance/1.3/PullToRefreshStyle.qml (+36/-17)
tests/unit_x11/tst_components/tst_pulltorefresh_listview.qml (+1/-1)
tests/unit_x11/tst_components/tst_pulltorefresh_pagestack_topmargin.qml (+86/-0)
tests/unit_x11/tst_components/tst_pulltorefresh_topmargin.qml (+59/-0)
To merge this branch: bzr merge lp:~tpeeters/ubuntu-ui-toolkit/refreshMargin
Reviewer Review Type Date Requested Status
ubuntu-sdk-build-bot continuous-integration Approve on 2016-05-13
Zsombor Egri (community) 2016-05-11 Approve on 2016-05-13
Review via email: mp+294436@code.launchpad.net

Commit Message

Fix flickable topMargin adjustments by PullToRefreshStyle.

To post a comment you must log in.
lp:~tpeeters/ubuntu-ui-toolkit/refreshMargin updated on 2016-05-11
1987. By Tim Peeters on 2016-05-11

clean

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~tpeeters/ubuntu-ui-toolkit/refreshMargin updated on 2016-05-11
1988. By Tim Peeters on 2016-05-11

clean

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
lp:~tpeeters/ubuntu-ui-toolkit/refreshMargin updated on 2016-05-11
1989. By Tim Peeters on 2016-05-11

fix license

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :

FAILED: Continuous integration, rev:1986
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/386/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/787/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/818/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/599/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/573/console
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3262/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3261
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3260
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3259
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3258

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/386/rebuild

review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :

FAILED: Continuous integration, rev:1989
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/387/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/788/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/819/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/600/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/574/console
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3267/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3266
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3265
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3264
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3263

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/387/rebuild

review: Needs Fixing (continuous-integration)
lp:~tpeeters/ubuntu-ui-toolkit/refreshMargin updated on 2016-05-12
1990. By Tim Peeters on 2016-05-12

add license

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :

PASSED: Continuous integration, rev:1990
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/389/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/790
        deb: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/790/artifact/artifacts/*zip*/artifacts.zip
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/821
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/602
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/576
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3278/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3277
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3274
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3275
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3276

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/389/rebuild

review: Approve (continuous-integration)
Zsombor Egri (zsombi) wrote :

A tiny fix and you're there :)

review: Needs Fixing
Alberto Mardegan (mardy) wrote :

Thanks Tim! I tested the artifacts from https://code.launchpad.net/~tpeeters/ubuntu-ui-toolkit/refreshMargin/+merge/294436/comments/755761 and it seems that the issue is completely solved.

lp:~tpeeters/ubuntu-ui-toolkit/refreshMargin updated on 2016-05-13
1991. By Tim Peeters on 2016-05-13

update imports

Zsombor Egri (zsombi) wrote :

All good, let's land it.

review: Approve
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :

PASSED: Continuous integration, rev:1991
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/393/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/794
        deb: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/794/artifact/artifacts/*zip*/artifacts.zip
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/825
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/606
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/580
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3299/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3298
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3294
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3295
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/3296

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/393/rebuild

review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/Ubuntu/Components/Themes/Ambiance/1.3/PullToRefreshStyle.qml'
2--- src/Ubuntu/Components/Themes/Ambiance/1.3/PullToRefreshStyle.qml 2015-12-16 11:53:01 +0000
3+++ src/Ubuntu/Components/Themes/Ambiance/1.3/PullToRefreshStyle.qml 2016-05-13 10:16:44 +0000
4@@ -1,5 +1,5 @@
5 /*
6- * Copyright 2014 Canonical Ltd.
7+ * Copyright 2016 Canonical Ltd.
8 *
9 * This program is free software; you can redistribute it and/or modify
10 * it under the terms of the GNU Lesser General Public License as published by
11@@ -56,16 +56,22 @@
12 Local properties
13 */
14 readonly property PullToRefresh control: styledItem
15- // property to store Flickable's toipMargin at the time the pull or auto-refresh is started
16- property real flickableTopMargin: 0.0
17+ // property to control adding of control.height to the Flickable's topMargin.
18+ property bool extendTopMargin: false
19+ onExtendTopMarginChanged: {
20+ if (extendTopMargin) {
21+ control.target.topMargin += control.height;
22+ } else {
23+ control.target.topMargin -= control.height;
24+ }
25+ }
26+
27 // store when the drag has happened at the beginning of the Flickable's content
28 property bool wasAtYBeginning: false
29 // initial contentY value when pull started
30 property real initialContentY: 0.0
31 // drives the refreshing state
32 property bool refreshing: false
33- // point of release used in rebind animation between the ready-to-refresh and refreshing states
34- property real pointOfRelease
35 // specifies the component completion
36 property bool ready: false
37 // root item
38@@ -130,6 +136,17 @@
39 anchors.centerIn: parent
40 }
41
42+ onVisibleChanged: {
43+ // Updates to contentY may be interrupted if the flickable becomes
44+ // invisible (for example when a new Page is pushed on a stack
45+ // while PullToRefresh is in refreshing state. See bug #1578619.
46+ if (visible) {
47+ // Make sure the flickable is back inside its bounds when
48+ // the Page becomes visible again:
49+ view.returnToBounds();
50+ }
51+ }
52+
53 // state and content controlling
54 Connections {
55 target: control
56@@ -138,8 +155,7 @@
57 return;
58 }
59 if (!style.releaseToRefresh && target.refreshing) {
60- // not a manual refresh, update flickable's starting topMargin
61- style.flickableTopMargin = control.target.topMargin;
62+ // not a manual refresh
63 style.wasAtYBeginning = control.target.atYBeginning;
64 }
65 /*
66@@ -162,13 +178,13 @@
67 style.refreshing = false;
68 style.releaseToRefresh = false;
69 }
70- onMovementEnded: style.wasAtYBeginning = control.target.atYBeginning
71+ onMovementEnded: {
72+ style.wasAtYBeginning = control.target.atYBeginning;
73+ }
74
75 // catch when to initiate refresh
76 onDraggingChanged: {
77 if (!control.parent.dragging && style.releaseToRefresh) {
78- pointOfRelease = -(control.target.contentY - control.target.originY)
79- style.flickableTopMargin = control.target.topMargin;
80 style.refreshing = true;
81 style.releaseToRefresh = false;
82 }
83@@ -213,8 +229,8 @@
84 running: true
85 }
86 PropertyChanges {
87- target: control.target
88- topMargin: style.flickableTopMargin + control.height
89+ target: style
90+ extendTopMargin: true
91 }
92 }
93 ]
94@@ -226,9 +242,8 @@
95 SequentialAnimation {
96 UbuntuNumberAnimation {
97 target: control.target
98- property: "topMargin"
99- from: style.pointOfRelease
100- to: style.flickableTopMargin + control.height
101+ property: "contentY"
102+ to: style.initialContentY - control.height
103 }
104 ScriptAction {
105 script: control.refresh()
106@@ -242,8 +257,7 @@
107 UbuntuNumberAnimation {
108 target: control.target
109 property: "contentY"
110- from: -style.flickableTopMargin
111- to: -style.flickableTopMargin - control.height
112+ to: style.initialContentY - control.height
113 }
114 },
115 Transition {
116@@ -253,6 +267,11 @@
117 target: control.target
118 property: "topMargin"
119 }
120+ UbuntuNumberAnimation {
121+ target: control.target
122+ property: "contentY"
123+ to: style.initialContentY
124+ }
125 }
126 ]
127 }
128
129=== modified file 'tests/unit_x11/tst_components/tst_pulltorefresh_listview.qml'
130--- tests/unit_x11/tst_components/tst_pulltorefresh_listview.qml 2015-03-03 13:20:06 +0000
131+++ tests/unit_x11/tst_components/tst_pulltorefresh_listview.qml 2016-05-13 10:16:44 +0000
132@@ -1,5 +1,5 @@
133 /*
134- * Copyright 2014 Canonical Ltd.
135+ * Copyright 2016 Canonical Ltd.
136 *
137 * This program is free software; you can redistribute it and/or modify
138 * it under the terms of the GNU Lesser General Public License as published by
139
140=== added file 'tests/unit_x11/tst_components/tst_pulltorefresh_pagestack_topmargin.qml'
141--- tests/unit_x11/tst_components/tst_pulltorefresh_pagestack_topmargin.qml 1970-01-01 00:00:00 +0000
142+++ tests/unit_x11/tst_components/tst_pulltorefresh_pagestack_topmargin.qml 2016-05-13 10:16:44 +0000
143@@ -0,0 +1,86 @@
144+/*
145+ * Copyright 2016 Canonical Ltd.
146+ *
147+ * This program is free software; you can redistribute it and/or modify
148+ * it under the terms of the GNU Lesser General Public License as published by
149+ * the Free Software Foundation; version 3.
150+ *
151+ * This program is distributed in the hope that it will be useful,
152+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
153+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
154+ * GNU Lesser General Public License for more details.
155+ *
156+ * You should have received a copy of the GNU Lesser General Public License
157+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
158+ */
159+
160+import QtQuick 2.4
161+import Ubuntu.Components 1.3
162+import Ubuntu.Components.ListItems 1.3
163+import QtTest 1.0
164+import Ubuntu.Test 1.3
165+
166+Item {
167+ width: units.gu(40)
168+ height: units.gu(71)
169+
170+ MainView {
171+ anchors.fill: parent
172+ PageStack {
173+ id: pageStack
174+ Page {
175+ id: page0
176+ title: "One"
177+ visible: false
178+
179+ ListView {
180+ id: view
181+ anchors.fill: parent
182+ model: 15
183+ delegate: Standard {
184+ width: ListView.view.width
185+ height: units.gu(5)
186+ text: index
187+ }
188+ PullToRefresh {
189+ id: pullMe
190+ onRefresh: {
191+ refreshing = true;
192+ refreshing = false;
193+ pageStack.push(page1)
194+ }
195+ }
196+ }
197+ }
198+ Page {
199+ id: page1
200+ title: "Two"
201+ visible: false
202+ Button {
203+ anchors.centerIn: parent
204+ onClicked: pageStack.pop()
205+ text: "back"
206+ }
207+ }
208+ Component.onCompleted: pageStack.push(page0)
209+ }
210+ }
211+
212+ UbuntuTestCase {
213+ name: "PullToRefreshPageStackPushWhileRefreshing"
214+ when: windowShown
215+
216+ function test_push_while_refreshing_bug1578619() {
217+ var initialTopMargin = view.topMargin;
218+ var initialContentY = view.contentY;
219+ flick(view, view.width/2, units.gu(10), 0, units.gu(40));
220+ wait(1000);
221+ pageStack.pop();
222+ tryCompare(view, "moving", false);
223+ tryCompare(view, "topMargin", initialTopMargin, 500,
224+ "Initial topMargin of flickable is not restored after refreshing.");
225+ tryCompare(view, "contentY", initialContentY, 500,
226+ "Initial contentY of flickable is not restored after refreshing.")
227+ }
228+ }
229+}
230
231=== added file 'tests/unit_x11/tst_components/tst_pulltorefresh_topmargin.qml'
232--- tests/unit_x11/tst_components/tst_pulltorefresh_topmargin.qml 1970-01-01 00:00:00 +0000
233+++ tests/unit_x11/tst_components/tst_pulltorefresh_topmargin.qml 2016-05-13 10:16:44 +0000
234@@ -0,0 +1,59 @@
235+/*
236+ * Copyright 2016 Canonical Ltd.
237+ *
238+ * This program is free software; you can redistribute it and/or modify
239+ * it under the terms of the GNU Lesser General Public License as published by
240+ * the Free Software Foundation; version 3.
241+ *
242+ * This program is distributed in the hope that it will be useful,
243+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
244+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
245+ * GNU Lesser General Public License for more details.
246+ *
247+ * You should have received a copy of the GNU Lesser General Public License
248+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
249+ */
250+
251+import QtQuick 2.4
252+import Ubuntu.Components 1.3
253+import QtTest 1.0
254+import Ubuntu.Test 1.3
255+
256+Item {
257+ width: units.gu(40)
258+ height: units.gu(71)
259+ id: root
260+ Flickable {
261+ id: view
262+ anchors.fill: parent
263+ contentHeight: 1.6*view.height
264+ Label {
265+ anchors.centerIn: parent
266+ text: "void"
267+ }
268+ PullToRefresh {
269+ id: pullToRefresh
270+ parent: view
271+ onRefresh: {
272+ refreshing = true;
273+ refreshing = false;
274+ }
275+ }
276+ }
277+
278+ UbuntuTestCase {
279+ name: "PullToRefreshTopMargin"
280+ when: windowShown
281+
282+ function test_reposition_after_refresh_bug1578619() {
283+ var initialTopMargin = view.topMargin;
284+ var initialContentY = view.contentY;
285+ flick(view, view.width/2, units.gu(10), 0, units.gu(40));
286+ tryCompare(view, "moving", false);
287+ tryCompare(view, "topMargin", initialTopMargin, 500,
288+ "Initial topMargin of flickable is not restored after refreshing.");
289+ tryCompare(view, "contentY", initialContentY, 500,
290+ "Initial contentY of flickable is not restored after refreshing.")
291+ }
292+ }
293+}

Subscribers

People subscribed via source and target branches