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

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 :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
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 :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
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
=== modified file 'plugins/ListViewWithPageHeader/listviewwithpageheader.cpp'
--- plugins/ListViewWithPageHeader/listviewwithpageheader.cpp 2013-11-19 04:53:47 +0000
+++ plugins/ListViewWithPageHeader/listviewwithpageheader.cpp 2013-11-19 08:37:05 +0000
@@ -477,7 +477,7 @@
477 qreal diff = m_previousContentY - contentY();477 qreal diff = m_previousContentY - contentY();
478 const bool showHeaderAnimationRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationShowHeader;478 const bool showHeaderAnimationRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationShowHeader;
479 if (m_headerItem) {479 if (m_headerItem) {
480 auto oldHeaderItemShownHeight = m_headerItemShownHeight;480 const auto oldHeaderItemShownHeight = m_headerItemShownHeight;
481 if (contentY() < -m_minYExtent) {481 if (contentY() < -m_minYExtent) {
482 // Stick the header item to the top when dragging down482 // Stick the header item to the top when dragging down
483 m_headerItem->setY(contentY());483 m_headerItem->setY(contentY());
@@ -492,9 +492,9 @@
492 const bool notShownByItsOwn = contentY() + diff >= m_headerItem->y() + m_headerItem->height();492 const bool notShownByItsOwn = contentY() + diff >= m_headerItem->y() + m_headerItem->height();
493 const bool maximizeVisibleAreaRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationMaximizeVisibleArea;493 const bool maximizeVisibleAreaRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationMaximizeVisibleArea;
494494
495 if (!scrolledUp && contentY() == -m_minYExtent) {495 if (!scrolledUp && (contentY() == -m_minYExtent || (m_headerItemShownHeight == 0 && m_previousContentY == m_headerItem->y()))) {
496 m_headerItemShownHeight = 0;496 m_headerItemShownHeight = 0;
497 m_headerItem->setY(contentY());497 m_headerItem->setY(-m_minYExtent);
498 } else if ((scrolledUp && notRebounding && notShownByItsOwn && !maximizeVisibleAreaRunning) || (m_headerItemShownHeight > 0) || m_inContentHeightKeepHeaderShown) {498 } else if ((scrolledUp && notRebounding && notShownByItsOwn && !maximizeVisibleAreaRunning) || (m_headerItemShownHeight > 0) || m_inContentHeightKeepHeaderShown) {
499 if (maximizeVisibleAreaRunning && diff > 0) // If we are maximizing and the header was shown, make sure we hide it499 if (maximizeVisibleAreaRunning && diff > 0) // If we are maximizing and the header was shown, make sure we hide it
500 m_headerItemShownHeight -= diff;500 m_headerItemShownHeight -= diff;
@@ -981,7 +981,6 @@
981 if (growUp) {981 if (growUp) {
982 ListItem *firstItem = m_visibleItems.first();982 ListItem *firstItem = m_visibleItems.first();
983 firstItem->setY(firstItem->y() - item->height());983 firstItem->setY(firstItem->y() - item->height());
984 adjustMinYExtent();
985 }984 }
986 // Adding an item may break a "same section" chain, so check985 // Adding an item may break a "same section" chain, so check
987 // if we need adding a new section item986 // if we need adding a new section item
@@ -992,7 +991,6 @@
992 if (growUp && nextItem->m_sectionItem) {991 if (growUp && nextItem->m_sectionItem) {
993 ListItem *firstItem = m_visibleItems.first();992 ListItem *firstItem = m_visibleItems.first();
994 firstItem->setY(firstItem->y() - nextItem->m_sectionItem->height());993 firstItem->setY(firstItem->y() - nextItem->m_sectionItem->height());
995 adjustMinYExtent();
996 }994 }
997 }995 }
998 }996 }
@@ -1001,6 +999,7 @@
1001 ListItem *firstItem = m_visibleItems.first();999 ListItem *firstItem = m_visibleItems.first();
1002 firstItem->setY(oldFirstValidIndexPos);1000 firstItem->setY(oldFirstValidIndexPos);
1003 }1001 }
1002 adjustMinYExtent();
1004 } else if (insert.index <= m_firstVisibleIndex) {1003 } else if (insert.index <= m_firstVisibleIndex) {
1005 m_firstVisibleIndex += insert.count;1004 m_firstVisibleIndex += insert.count;
1006 }1005 }
10071006
=== modified file 'tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp'
--- tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp 2013-11-18 08:33:33 +0000
+++ tests/plugins/ListViewWithPageHeader/listviewwithpageheadertest.cpp 2013-11-19 08:37:05 +0000
@@ -1589,6 +1589,28 @@
1589 QCOMPARE(lvwph->m_headerItemShownHeight, 0.);1589 QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
1590 }1590 }
15911591
1592 void testMaximizeVisibleAreaTopWithHalfPageHeaderUpDown()
1593 {
1594 testMaximizeVisibleAreaTopWithHalfPageHeader();
1595
1596 changeContentY(-60);
1597 changeContentY(15);
1598
1599 QTRY_COMPARE(lvwph->m_visibleItems.count(), 4);
1600 QCOMPARE(lvwph->m_firstVisibleIndex, 0);
1601 verifyItem(0, -140., 150., false);
1602 verifyItem(1, 10., 200., false);
1603 verifyItem(2, 210, 350., false);
1604 verifyItem(3, 560, 350., true);
1605 QCOMPARE(lvwph->m_minYExtent, 0.);
1606 QCOMPARE(lvwph->m_clipItem->y(), 190.);
1607 QCOMPARE(lvwph->m_clipItem->clip(), true);
1608 QCOMPARE(lvwph->m_headerItem->y(), 140.);
1609 QCOMPARE(lvwph->m_headerItem->height(), 50.);
1610 QCOMPARE(lvwph->contentY(), 155.);
1611 QCOMPARE(lvwph->m_headerItemShownHeight, 35.);
1612 }
1613
1592 void testMaximizeVisibleAreaBottomWithHalfPageHeader()1614 void testMaximizeVisibleAreaBottomWithHalfPageHeader()
1593 {1615 {
1594 changeContentY(430);1616 changeContentY(430);
15951617
=== modified file 'tests/plugins/ListViewWithPageHeader/listviewwithpageheadertestsection.cpp'
--- tests/plugins/ListViewWithPageHeader/listviewwithpageheadertestsection.cpp 2013-09-02 13:26:12 +0000
+++ tests/plugins/ListViewWithPageHeader/listviewwithpageheadertestsection.cpp 2013-11-19 08:37:05 +0000
@@ -1734,7 +1734,25 @@
1734 QMetaObject::invokeMethod(model, "insertItem", Q_ARG(QVariant, 0), Q_ARG(QVariant, 50), Q_ARG(QVariant, "Agressive"));1734 QMetaObject::invokeMethod(model, "insertItem", Q_ARG(QVariant, 0), Q_ARG(QVariant, 50), Q_ARG(QVariant, "Agressive"));
1735 QMetaObject::invokeMethod(model, "insertItem", Q_ARG(QVariant, 0), Q_ARG(QVariant, 50), Q_ARG(QVariant, "Agressive"));1735 QMetaObject::invokeMethod(model, "insertItem", Q_ARG(QVariant, 0), Q_ARG(QVariant, 50), Q_ARG(QVariant, "Agressive"));
17361736
1737 changeContentY(140);1737 changeContentY(5);
1738
1739 QTRY_COMPARE(lvwph->m_visibleItems.count(), 5);
1740 QCOMPARE(lvwph->m_firstVisibleIndex, 0);
1741 verifyItem(0, 45., 90., false, "Agressive", false);
1742 verifyItem(1, 135., 50., false, QString(), false);
1743 verifyItem(2, 185., 150., false, QString(), false);
1744 verifyItem(3, 335., 240., false, "Regular", false);
1745 verifyItem(4, 575., 390., true, "Mild", true);
1746 QCOMPARE(lvwph->m_minYExtent, 0.);
1747 QCOMPARE(lvwph->m_clipItem->y(), 5.);
1748 QCOMPARE(lvwph->m_clipItem->clip(), false);
1749 QCOMPARE(lvwph->m_headerItem->y(), 0.);
1750 QCOMPARE(lvwph->m_headerItem->height(), 50.);
1751 QCOMPARE(lvwph->contentY(), 5.);
1752 QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
1753 QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
1754
1755 changeContentY(135);
17381756
1739 QTRY_COMPARE(lvwph->m_visibleItems.count(), 5);1757 QTRY_COMPARE(lvwph->m_visibleItems.count(), 5);
1740 QCOMPARE(lvwph->m_firstVisibleIndex, 0);1758 QCOMPARE(lvwph->m_firstVisibleIndex, 0);
@@ -2076,6 +2094,68 @@
2076 QCOMPARE(spy.count(), 0);2094 QCOMPARE(spy.count(), 0);
2077 }2095 }
20782096
2097 void testDownAndUp()
2098 {
2099 QMetaObject::invokeMethod(model, "removeItems", Q_ARG(QVariant, 1), Q_ARG(QVariant, 5));
2100
2101 QTRY_COMPARE(lvwph->m_visibleItems.count(), 1);
2102 QCOMPARE(lvwph->m_firstVisibleIndex, 0);
2103 verifyItem(0, 50., 190., false, "Agressive", false);
2104 QCOMPARE(lvwph->m_minYExtent, 0.);
2105 QCOMPARE(lvwph->m_clipItem->y(), 0.);
2106 QCOMPARE(lvwph->m_clipItem->clip(), false);
2107 QCOMPARE(lvwph->m_headerItem->y(), 0.);
2108 QCOMPARE(lvwph->m_headerItem->height(), 50.);
2109 QCOMPARE(lvwph->contentY(), 0.);
2110 QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
2111 QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
2112
2113 QTest::qWait(1);
2114
2115 changeContentY(-15);
2116
2117 QTRY_COMPARE(lvwph->m_visibleItems.count(), 1);
2118 QCOMPARE(lvwph->m_firstVisibleIndex, 0);
2119 verifyItem(0, 65., 190., false, "Agressive", false);
2120 QCOMPARE(lvwph->m_minYExtent, 0.);
2121 QCOMPARE(lvwph->m_clipItem->y(), -15.);
2122 QCOMPARE(lvwph->m_clipItem->clip(), false);
2123 QCOMPARE(lvwph->m_headerItem->y(), -15.);
2124 QCOMPARE(lvwph->m_headerItem->height(), 65.);
2125 QCOMPARE(lvwph->contentY(), -15.);
2126 QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
2127 QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
2128
2129 changeContentY(25);
2130
2131 QTRY_COMPARE(lvwph->m_visibleItems.count(), 1);
2132 QCOMPARE(lvwph->m_firstVisibleIndex, 0);
2133 verifyItem(0, 40., 190., false, "Agressive", false);
2134 QCOMPARE(lvwph->m_minYExtent, 0.);
2135 QCOMPARE(lvwph->m_clipItem->y(), 10.);
2136 QCOMPARE(lvwph->m_clipItem->clip(), false);
2137 QCOMPARE(lvwph->m_headerItem->y(), 0.);
2138 QCOMPARE(lvwph->m_headerItem->height(), 50.);
2139 QCOMPARE(lvwph->contentY(), 10.);
2140 QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
2141 QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
2142
2143
2144 QTest::mouseClick(view, Qt::LeftButton, Qt::NoModifier, QPoint(0, 0));
2145
2146 QTRY_COMPARE(lvwph->m_visibleItems.count(), 1);
2147 QCOMPARE(lvwph->m_firstVisibleIndex, 0);
2148 verifyItem(0, 50., 190., false, "Agressive", false);
2149 QCOMPARE(lvwph->m_minYExtent, 0.);
2150 QCOMPARE(lvwph->m_clipItem->y(), 0.);
2151 QCOMPARE(lvwph->m_clipItem->clip(), false);
2152 QCOMPARE(lvwph->m_headerItem->y(), 0.);
2153 QCOMPARE(lvwph->m_headerItem->height(), 50.);
2154 QCOMPARE(lvwph->contentY(), 0.);
2155 QCOMPARE(lvwph->m_headerItemShownHeight, 0.);
2156 QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
2157 }
2158
2079private:2159private:
2080 QQuickView *view;2160 QQuickView *view;
2081 ListViewWithPageHeader *lvwph;2161 ListViewWithPageHeader *lvwph;

Subscribers

People subscribed via source and target branches