Unity 8

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

  1. lvwph_bad_header_position_1245824
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Michał Sawicz
Approved revision: 499
Merged at revision: 538
Proposed branch: lp:~aacid/unity8/lvwph_bad_header_position_1245824
Merge into: lp:unity8
Diff against target: 181 lines (+107/-6)
3 files modified
plugins/ListViewWithPageHeader/listviewwithpageheader.cpp (+4/-5)
tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp (+22/-0)
tests/plugins/ListViewWithPageHeader/listviewwithpageheadertestsection.cpp (+81/-1)
To merge this branch: bzr merge lp:~aacid/unity8/lvwph_bad_header_position_1245824
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Nick Dedekind (community) Approve
Review via email: mp+193754@code.launchpad.net

Commit message

Fix header getting lost as per bug 1245824

To post a comment you must log in.
Revision history for this message
Michael Zanetti (mzanetti) wrote :

yes! fixes the issue for me!

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

FAILED: Continuous integration, rev:492
http://jenkins.qa.ubuntu.com/job/unity8-ci/1546/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/339
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/327
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/90
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/70
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/70
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/70/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/70
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/320
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/339
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/339/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/327
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/327/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/2996
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/919

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1546/rebuild

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

PASSED: Continuous integration, rev:498
http://jenkins.qa.ubuntu.com/job/unity8-ci/1567/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/415
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/403
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/122
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/91
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/91
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/91/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/91
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/387
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/415
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/415/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/403
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/403/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3067
    SUCCESS: http://10.97.0.26:8080/job/touch-flash-device/1027

Click here to trigger a rebuild:
http://10.97.0.26:8080/job/unity8-ci/1567/rebuild

review: Approve (continuous-integration)
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

testMaximizeVisibleAreaTopWithHalfPageHeaderUpDown succeeds under both trunk and this branch.
Is this a validation of a previous assumption or testing something broken that was fixed?

review: Needs Information
Revision history for this message
Albert Astals Cid (aacid) wrote :

Yes, that is the point.

If you read the log you can see that in r493 I introduce testMaximizeVisibleAreaTopWithHalfPageHeaderUpDown with "This new test is failing at the moment, that is correct since highlights a regression caused by this branch" and then in r498 I introduce testDownAndUp that is "the test that shows why the fix is needed".

Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> Yes, that is the point.
>
> If you read the log you can see that in r493 I introduce
> testMaximizeVisibleAreaTopWithHalfPageHeaderUpDown with "This new test is
> failing at the moment, that is correct since highlights a regression caused by
> this branch" and then in r498 I introduce testDownAndUp that is "the test that
> shows why the fix is needed".

Ok thanks.
LGTM

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/696/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/generic-cleanup-mbs/3221/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/747
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/735
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/258
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/82
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/82
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/82/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/82
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/681
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/747
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/747/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/735
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/735/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3364
    SUCCESS: http://s-jenkins:8080/job/touch-flash-device/1453

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/712/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/generic-cleanup-mbs/3256/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/829/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/817/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/289/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/98/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/98/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/98/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/829/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/817/console

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

FAILED: Continuous integration, rev:499
http://jenkins.qa.ubuntu.com/job/unity8-ci/1670/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/836
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/824/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/296
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/193
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/194
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/194/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/193
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/753
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/836
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/836/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/824/console

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

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/721/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3287
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/862
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/850
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/301
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/107
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/107
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/107/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/107
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/774
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/862
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/862/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/850
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/850/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3452
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/1541

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
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/ListViewWithPageHeader/listviewwithpageheader.cpp'
2--- plugins/ListViewWithPageHeader/listviewwithpageheader.cpp 2013-11-19 04:53:47 +0000
3+++ plugins/ListViewWithPageHeader/listviewwithpageheader.cpp 2013-11-19 08:37:05 +0000
4@@ -477,7 +477,7 @@
5 qreal diff = m_previousContentY - contentY();
6 const bool showHeaderAnimationRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationShowHeader;
7 if (m_headerItem) {
8- auto oldHeaderItemShownHeight = m_headerItemShownHeight;
9+ const auto oldHeaderItemShownHeight = m_headerItemShownHeight;
10 if (contentY() < -m_minYExtent) {
11 // Stick the header item to the top when dragging down
12 m_headerItem->setY(contentY());
13@@ -492,9 +492,9 @@
14 const bool notShownByItsOwn = contentY() + diff >= m_headerItem->y() + m_headerItem->height();
15 const bool maximizeVisibleAreaRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationMaximizeVisibleArea;
16
17- if (!scrolledUp && contentY() == -m_minYExtent) {
18+ if (!scrolledUp && (contentY() == -m_minYExtent || (m_headerItemShownHeight == 0 && m_previousContentY == m_headerItem->y()))) {
19 m_headerItemShownHeight = 0;
20- m_headerItem->setY(contentY());
21+ m_headerItem->setY(-m_minYExtent);
22 } else if ((scrolledUp && notRebounding && notShownByItsOwn && !maximizeVisibleAreaRunning) || (m_headerItemShownHeight > 0) || m_inContentHeightKeepHeaderShown) {
23 if (maximizeVisibleAreaRunning && diff > 0) // If we are maximizing and the header was shown, make sure we hide it
24 m_headerItemShownHeight -= diff;
25@@ -981,7 +981,6 @@
26 if (growUp) {
27 ListItem *firstItem = m_visibleItems.first();
28 firstItem->setY(firstItem->y() - item->height());
29- adjustMinYExtent();
30 }
31 // Adding an item may break a "same section" chain, so check
32 // if we need adding a new section item
33@@ -992,7 +991,6 @@
34 if (growUp && nextItem->m_sectionItem) {
35 ListItem *firstItem = m_visibleItems.first();
36 firstItem->setY(firstItem->y() - nextItem->m_sectionItem->height());
37- adjustMinYExtent();
38 }
39 }
40 }
41@@ -1001,6 +999,7 @@
42 ListItem *firstItem = m_visibleItems.first();
43 firstItem->setY(oldFirstValidIndexPos);
44 }
45+ adjustMinYExtent();
46 } else if (insert.index <= m_firstVisibleIndex) {
47 m_firstVisibleIndex += insert.count;
48 }
49
50=== modified file 'tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp'
51--- tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp 2013-11-18 08:33:33 +0000
52+++ tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp 2013-11-19 08:37:05 +0000
53@@ -1589,6 +1589,28 @@
54 QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
55 }
56
57+ void testMaximizeVisibleAreaTopWithHalfPageHeaderUpDown()
58+ {
59+ testMaximizeVisibleAreaTopWithHalfPageHeader();
60+
61+ changeContentY(-60);
62+ changeContentY(15);
63+
64+ QTRY_COMPARE(lvwph->m_visibleItems.count(), 4);
65+ QCOMPARE(lvwph->m_firstVisibleIndex, 0);
66+ verifyItem(0, -140., 150., false);
67+ verifyItem(1, 10., 200., false);
68+ verifyItem(2, 210, 350., false);
69+ verifyItem(3, 560, 350., true);
70+ QCOMPARE(lvwph->m_minYExtent, 0.);
71+ QCOMPARE(lvwph->m_clipItem->y(), 190.);
72+ QCOMPARE(lvwph->m_clipItem->clip(), true);
73+ QCOMPARE(lvwph->m_headerItem->y(), 140.);
74+ QCOMPARE(lvwph->m_headerItem->height(), 50.);
75+ QCOMPARE(lvwph->contentY(), 155.);
76+ QCOMPARE(lvwph->m_headerItemShownHeight, 35.);
77+ }
78+
79 void testMaximizeVisibleAreaBottomWithHalfPageHeader()
80 {
81 changeContentY(430);
82
83=== modified file 'tests/plugins/ListViewWithPageHeader/listviewwithpageheadertestsection.cpp'
84--- tests/plugins/ListViewWithPageHeader/listviewwithpageheadertestsection.cpp 2013-09-02 13:26:12 +0000
85+++ tests/plugins/ListViewWithPageHeader/listviewwithpageheadertestsection.cpp 2013-11-19 08:37:05 +0000
86@@ -1734,7 +1734,25 @@
87 QMetaObject::invokeMethod(model, "insertItem", Q_ARG(QVariant, 0), Q_ARG(QVariant, 50), Q_ARG(QVariant, "Agressive"));
88 QMetaObject::invokeMethod(model, "insertItem", Q_ARG(QVariant, 0), Q_ARG(QVariant, 50), Q_ARG(QVariant, "Agressive"));
89
90- changeContentY(140);
91+ changeContentY(5);
92+
93+ QTRY_COMPARE(lvwph->m_visibleItems.count(), 5);
94+ QCOMPARE(lvwph->m_firstVisibleIndex, 0);
95+ verifyItem(0, 45., 90., false, "Agressive", false);
96+ verifyItem(1, 135., 50., false, QString(), false);
97+ verifyItem(2, 185., 150., false, QString(), false);
98+ verifyItem(3, 335., 240., false, "Regular", false);
99+ verifyItem(4, 575., 390., true, "Mild", true);
100+ QCOMPARE(lvwph->m_minYExtent, 0.);
101+ QCOMPARE(lvwph->m_clipItem->y(), 5.);
102+ QCOMPARE(lvwph->m_clipItem->clip(), false);
103+ QCOMPARE(lvwph->m_headerItem->y(), 0.);
104+ QCOMPARE(lvwph->m_headerItem->height(), 50.);
105+ QCOMPARE(lvwph->contentY(), 5.);
106+ QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
107+ QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
108+
109+ changeContentY(135);
110
111 QTRY_COMPARE(lvwph->m_visibleItems.count(), 5);
112 QCOMPARE(lvwph->m_firstVisibleIndex, 0);
113@@ -2076,6 +2094,68 @@
114 QCOMPARE(spy.count(), 0);
115 }
116
117+ void testDownAndUp()
118+ {
119+ QMetaObject::invokeMethod(model, "removeItems", Q_ARG(QVariant, 1), Q_ARG(QVariant, 5));
120+
121+ QTRY_COMPARE(lvwph->m_visibleItems.count(), 1);
122+ QCOMPARE(lvwph->m_firstVisibleIndex, 0);
123+ verifyItem(0, 50., 190., false, "Agressive", false);
124+ QCOMPARE(lvwph->m_minYExtent, 0.);
125+ QCOMPARE(lvwph->m_clipItem->y(), 0.);
126+ QCOMPARE(lvwph->m_clipItem->clip(), false);
127+ QCOMPARE(lvwph->m_headerItem->y(), 0.);
128+ QCOMPARE(lvwph->m_headerItem->height(), 50.);
129+ QCOMPARE(lvwph->contentY(), 0.);
130+ QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
131+ QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
132+
133+ QTest::qWait(1);
134+
135+ changeContentY(-15);
136+
137+ QTRY_COMPARE(lvwph->m_visibleItems.count(), 1);
138+ QCOMPARE(lvwph->m_firstVisibleIndex, 0);
139+ verifyItem(0, 65., 190., false, "Agressive", false);
140+ QCOMPARE(lvwph->m_minYExtent, 0.);
141+ QCOMPARE(lvwph->m_clipItem->y(), -15.);
142+ QCOMPARE(lvwph->m_clipItem->clip(), false);
143+ QCOMPARE(lvwph->m_headerItem->y(), -15.);
144+ QCOMPARE(lvwph->m_headerItem->height(), 65.);
145+ QCOMPARE(lvwph->contentY(), -15.);
146+ QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
147+ QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
148+
149+ changeContentY(25);
150+
151+ QTRY_COMPARE(lvwph->m_visibleItems.count(), 1);
152+ QCOMPARE(lvwph->m_firstVisibleIndex, 0);
153+ verifyItem(0, 40., 190., false, "Agressive", false);
154+ QCOMPARE(lvwph->m_minYExtent, 0.);
155+ QCOMPARE(lvwph->m_clipItem->y(), 10.);
156+ QCOMPARE(lvwph->m_clipItem->clip(), false);
157+ QCOMPARE(lvwph->m_headerItem->y(), 0.);
158+ QCOMPARE(lvwph->m_headerItem->height(), 50.);
159+ QCOMPARE(lvwph->contentY(), 10.);
160+ QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
161+ QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
162+
163+
164+ QTest::mouseClick(view, Qt::LeftButton, Qt::NoModifier, QPoint(0, 0));
165+
166+ QTRY_COMPARE(lvwph->m_visibleItems.count(), 1);
167+ QCOMPARE(lvwph->m_firstVisibleIndex, 0);
168+ verifyItem(0, 50., 190., false, "Agressive", false);
169+ QCOMPARE(lvwph->m_minYExtent, 0.);
170+ QCOMPARE(lvwph->m_clipItem->y(), 0.);
171+ QCOMPARE(lvwph->m_clipItem->clip(), false);
172+ QCOMPARE(lvwph->m_headerItem->y(), 0.);
173+ QCOMPARE(lvwph->m_headerItem->height(), 50.);
174+ QCOMPARE(lvwph->contentY(), 0.);
175+ QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
176+ QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
177+ }
178+
179 private:
180 QQuickView *view;
181 ListViewWithPageHeader *lvwph;

Subscribers

People subscribed via source and target branches