Merge lp:~tpeeters/ubuntu-ui-toolkit/10-Header-animate into lp:ubuntu-ui-toolkit/staging

Proposed by Tim Peeters
Status: Rejected
Rejected by: Tim Peeters
Proposed branch: lp:~tpeeters/ubuntu-ui-toolkit/10-Header-animate
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 176 lines (+65/-7)
4 files modified
components.api (+1/-0)
src/Ubuntu/Components/plugin/ucheader.cpp (+29/-2)
src/Ubuntu/Components/plugin/ucheader.h (+4/-0)
tests/unit_x11/tst_components/tst_header.qml (+31/-5)
To merge this branch: bzr merge lp:~tpeeters/ubuntu-ui-toolkit/10-Header-animate
Reviewer Review Type Date Requested Status
Tim Peeters Disapprove
PS Jenkins bot continuous-integration Approve
Review via email: mp+272422@code.launchpad.net

Commit message

Add animate property to cpp Header.

Description of the change

Add animate property to cpp Header. I added this property because the current behavior of the header is to always show without animating it in, after switching between apps.

To post a comment you must log in.
1655. By Tim Peeters

document default value of animate

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
1656. By Tim Peeters

update unit tests

1657. By Tim Peeters

update components.api

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

Don't happrove this yet. We'll have a talk on Wednesday whether it is ever needed to disable animating of the header.

review: Needs Information
Revision history for this message
Tim Peeters (tpeeters) wrote :

If this bug is valid https://bugs.launchpad.net/ubuntu-ux/+bug/1500483 then we don't need the animate property.

Revision history for this message
Tim Peeters (tpeeters) wrote :

The header needs to animate when showing on app focus. MR for the current (old) Header here: https://code.launchpad.net/~tpeeters/ubuntu-ui-toolkit/11-animate-header-on-app-focus/+merge/272766

New header will have the same behavior.

review: Disapprove

Unmerged revisions

1657. By Tim Peeters

update components.api

1656. By Tim Peeters

update unit tests

1655. By Tim Peeters

document default value of animate

1654. By Tim Peeters

add animate property

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'components.api'
--- components.api 2015-09-24 20:03:02 +0000
+++ components.api 2015-09-28 13:41:26 +0000
@@ -448,6 +448,7 @@
448Ubuntu.Components.ListItems.Header 1.3: Item448Ubuntu.Components.ListItems.Header 1.3: Item
449 property string text449 property string text
450Ubuntu.Components.Header 1.3: StyledItem450Ubuntu.Components.Header 1.3: StyledItem
451 property bool animate
451 property bool exposed452 property bool exposed
452 property Flickable flickable453 property Flickable flickable
453 readonly property bool moving454 readonly property bool moving
454455
=== modified file 'src/Ubuntu/Components/plugin/ucheader.cpp'
--- src/Ubuntu/Components/plugin/ucheader.cpp 2015-09-23 11:35:33 +0000
+++ src/Ubuntu/Components/plugin/ucheader.cpp 2015-09-28 13:41:26 +0000
@@ -87,6 +87,7 @@
87 , m_previous_contentY(0)87 , m_previous_contentY(0)
88 , m_exposed(true)88 , m_exposed(true)
89 , m_moving(false)89 , m_moving(false)
90 , m_animate(true)
90{91{
91 m_showHideAnimation->setParent(this);92 m_showHideAnimation->setParent(this);
92 m_showHideAnimation->setTargetObject(this);93 m_showHideAnimation->setTargetObject(this);
@@ -202,7 +203,7 @@
202 m_showHideAnimation->stop();203 m_showHideAnimation->stop();
203 }204 }
204 }205 }
205 if (isComponentComplete()) {206 if (m_animate && isComponentComplete()) {
206 m_showHideAnimation->setFrom(y());207 m_showHideAnimation->setFrom(y());
207 m_showHideAnimation->setTo(0.0);208 m_showHideAnimation->setTo(0.0);
208 m_showHideAnimation->start();209 m_showHideAnimation->start();
@@ -224,7 +225,7 @@
224 m_showHideAnimation->stop();225 m_showHideAnimation->stop();
225 }226 }
226 }227 }
227 if (isComponentComplete()) {228 if (m_animate && isComponentComplete()) {
228 m_showHideAnimation->setFrom(y());229 m_showHideAnimation->setFrom(y());
229 m_showHideAnimation->setTo(-1.0*height());230 m_showHideAnimation->setTo(-1.0*height());
230 m_showHideAnimation->start();231 m_showHideAnimation->start();
@@ -277,6 +278,32 @@
277 return m_moving;278 return m_moving;
278}279}
279280
281/*!
282 * \qmlproperty bool Header::animate
283 * Enable or disable animating of the header when exposing or hiding it
284 * either by setting the \l exposed property, or when the user finishes
285 * interaction with the \l flickable. Default value: true.
286 */
287void UCHeader::setAnimate(bool animate) {
288 if (m_animate == animate) {
289 return;
290 }
291
292 m_animate = animate;
293 if (!m_animate && m_moving) {
294 m_showHideAnimation->stop();
295 // _q_showHideAnimationRunningChanged() will update m_moving.
296
297 // Finish showing/hiding:
298 if (m_exposed) {
299 show();
300 } else {
301 hide();
302 }
303 }
304 Q_EMIT animateChanged();
305}
306
280// Called when moving due to user interaction with the flickable, or by307// Called when moving due to user interaction with the flickable, or by
281// setting m_flickable.contentY programatically.308// setting m_flickable.contentY programatically.
282void UCHeader::_q_scrolledContents() {309void UCHeader::_q_scrolledContents() {
283310
=== modified file 'src/Ubuntu/Components/plugin/ucheader.h'
--- src/Ubuntu/Components/plugin/ucheader.h 2015-09-22 18:24:27 +0000
+++ src/Ubuntu/Components/plugin/ucheader.h 2015-09-28 13:41:26 +0000
@@ -31,6 +31,7 @@
31 Q_PROPERTY(QQuickFlickable* flickable READ flickable WRITE setFlickable NOTIFY flickableChanged FINAL)31 Q_PROPERTY(QQuickFlickable* flickable READ flickable WRITE setFlickable NOTIFY flickableChanged FINAL)
32 Q_PROPERTY(bool exposed MEMBER m_exposed WRITE setExposed NOTIFY exposedChanged FINAL)32 Q_PROPERTY(bool exposed MEMBER m_exposed WRITE setExposed NOTIFY exposedChanged FINAL)
33 Q_PROPERTY(bool moving READ moving NOTIFY movingChanged FINAL)33 Q_PROPERTY(bool moving READ moving NOTIFY movingChanged FINAL)
34 Q_PROPERTY(bool animate MEMBER m_animate WRITE setAnimate NOTIFY animateChanged FINAL)
3435
35public:36public:
36 explicit UCHeader(QQuickItem *parent = 0);37 explicit UCHeader(QQuickItem *parent = 0);
@@ -40,11 +41,13 @@
40 void setFlickable(QQuickFlickable* flickable);41 void setFlickable(QQuickFlickable* flickable);
41 void setExposed(bool exposed);42 void setExposed(bool exposed);
42 bool moving();43 bool moving();
44 void setAnimate(bool animate);
4345
44Q_SIGNALS:46Q_SIGNALS:
45 void flickableChanged();47 void flickableChanged();
46 void exposedChanged();48 void exposedChanged();
47 void movingChanged();49 void movingChanged();
50 void animateChanged();
4851
49protected:52protected:
50 void show();53 void show();
@@ -66,6 +69,7 @@
66 qreal m_previous_contentY;69 qreal m_previous_contentY;
67 bool m_exposed:1;70 bool m_exposed:1;
68 bool m_moving:1;71 bool m_moving:1;
72 bool m_animate:1;
6973
70 // used to set the easing and duration of m_showHideAnimation74 // used to set the easing and duration of m_showHideAnimation
71 static UCUbuntuAnimation *s_ubuntuAnimation;75 static UCUbuntuAnimation *s_ubuntuAnimation;
7276
=== modified file 'tests/unit_x11/tst_components/tst_header.qml'
--- tests/unit_x11/tst_components/tst_header.qml 2015-09-22 14:13:36 +0000
+++ tests/unit_x11/tst_components/tst_header.qml 2015-09-28 13:41:26 +0000
@@ -82,9 +82,15 @@
82 Label {82 Label {
83 text: "header exposed"83 text: "header exposed"
84 }84 }
85 Item {85 Switch {
86 width: 186 id: animateSwitch
87 height: 187 checked: header.animate
88 function trigger() {
89 header.animate = !header.animate;
90 }
91 }
92 Label {
93 text: "animate header"
88 }94 }
89 }95 }
90 Button {96 Button {
@@ -163,6 +169,7 @@
163 // note: moving may be true briefly due to header height changes, but169 // note: moving may be true briefly due to header height changes, but
164 // it does not change in the initialization after wait_for_exposed() above.170 // it does not change in the initialization after wait_for_exposed() above.
165 compare(header.moving, false, "Header moving initially.");171 compare(header.moving, false, "Header moving initially.");
172 compare(header.animate, true, "Header does not animate by default.");
166 }173 }
167174
168 function init() {175 function init() {
@@ -268,9 +275,11 @@
268275
269 function test_set_exposed_to_hide_and_show() {276 function test_set_exposed_to_hide_and_show() {
270 header.exposed = false;277 header.exposed = false;
271 wait_for_exposed(false, "Cannot hide header by setting visible to false.");278 tryCompare(header, "moving", true, 1000, "Header does not start moving after setting exposed to false.");
279 wait_for_exposed(false, "Cannot hide header by setting exposed to false.");
272 header.exposed = true;280 header.exposed = true;
273 wait_for_exposed(true, "Cannot show header by setting visible to true.");281 tryCompare(header, "moving", true, 1000, "Header does not start moving after setting exposed to true.");
282 wait_for_exposed(true, "Cannot show header by setting exposed to true.");
274283
275 // change the value of exposed twice quickly:284 // change the value of exposed twice quickly:
276 header.exposed = false;285 header.exposed = false;
@@ -350,5 +359,22 @@
350359
351 header.flickable = flickable;360 header.flickable = flickable;
352 }361 }
362
363 function test_animate_off() {
364 header.animate = false;
365 compare(header.animate, false, "Cannot disable header animation.");
366 compare(header.moving, false, "Disabling animate sets moving.");
367 header.exposed = false;
368 compare(header.moving, false, "Header moves while animate is off.");
369 compare(header.exposed, false, "Header exposed is not unset instantly with animate off.");
370 compare(header.y, -header.height, "Header is not instantly hidden by setting exposed to false with animate off.");
371
372 header.exposed = true;
373 compare(header.moving, false, "Header moves after exposing it with animate off.");
374 compare(header.exposed, true, "Header exposed is not set instantly with animate off.");
375 compare(header.y, 0, "Header is not instantly shown by setting exposed with animate off.");
376
377 header.animate = true;
378 }
353 }379 }
354}380}

Subscribers

People subscribed via source and target branches