Merge lp:~aacid/unity8/lvwph_bad_header_position_1245824 into lp:unity8
- lvwph_bad_header_position_1245824
- Merge into trunk
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 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
PS Jenkins bot (community) | continuous-integration | Approve | |
Nick Dedekind (community) | Approve | ||
Review via email:
|
Commit message
Fix header getting lost as per bug 1245824
Description of the change
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Zanetti (mzanetti) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:492
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
PASSED: Continuous integration, rev:498
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Nick Dedekind (nick-dedekind) wrote : | # |
testMaximizeVis
Is this a validation of a previous assumption or testing something broken that was fixed?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Albert Astals Cid (aacid) wrote : | # |
Yes, that is the point.
If you read the log you can see that in r493 I introduce testMaximizeVis
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Nick Dedekind (nick-dedekind) wrote : | # |
> Yes, that is the point.
>
> If you read the log you can see that in r493 I introduce
> testMaximizeVis
> 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
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
FAILURE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
FAILURE: http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:499
http://
Executed test runs:
UNSTABLE: http://
FAILURE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
FAILURE: http://
Click here to trigger a rebuild:
http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Autolanding.
More details in the following jenkins job:
http://
Executed test runs:
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
SUCCESS: http://
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
PS Jenkins bot (ps-jenkins) : | # |
Preview Diff
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 | 477 | qreal diff = m_previousContentY - contentY(); | 477 | qreal diff = m_previousContentY - contentY(); |
6 | 478 | const bool showHeaderAnimationRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationShowHeader; | 478 | const bool showHeaderAnimationRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationShowHeader; |
7 | 479 | if (m_headerItem) { | 479 | if (m_headerItem) { |
9 | 480 | auto oldHeaderItemShownHeight = m_headerItemShownHeight; | 480 | const auto oldHeaderItemShownHeight = m_headerItemShownHeight; |
10 | 481 | if (contentY() < -m_minYExtent) { | 481 | if (contentY() < -m_minYExtent) { |
11 | 482 | // Stick the header item to the top when dragging down | 482 | // Stick the header item to the top when dragging down |
12 | 483 | m_headerItem->setY(contentY()); | 483 | m_headerItem->setY(contentY()); |
13 | @@ -492,9 +492,9 @@ | |||
14 | 492 | const bool notShownByItsOwn = contentY() + diff >= m_headerItem->y() + m_headerItem->height(); | 492 | const bool notShownByItsOwn = contentY() + diff >= m_headerItem->y() + m_headerItem->height(); |
15 | 493 | const bool maximizeVisibleAreaRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationMaximizeVisibleArea; | 493 | const bool maximizeVisibleAreaRunning = m_contentYAnimation->isRunning() && contentYAnimationType == ContentYAnimationMaximizeVisibleArea; |
16 | 494 | 494 | ||
18 | 495 | if (!scrolledUp && contentY() == -m_minYExtent) { | 495 | if (!scrolledUp && (contentY() == -m_minYExtent || (m_headerItemShownHeight == 0 && m_previousContentY == m_headerItem->y()))) { |
19 | 496 | m_headerItemShownHeight = 0; | 496 | m_headerItemShownHeight = 0; |
21 | 497 | m_headerItem->setY(contentY()); | 497 | m_headerItem->setY(-m_minYExtent); |
22 | 498 | } else if ((scrolledUp && notRebounding && notShownByItsOwn && !maximizeVisibleAreaRunning) || (m_headerItemShownHeight > 0) || m_inContentHeightKeepHeaderShown) { | 498 | } else if ((scrolledUp && notRebounding && notShownByItsOwn && !maximizeVisibleAreaRunning) || (m_headerItemShownHeight > 0) || m_inContentHeightKeepHeaderShown) { |
23 | 499 | if (maximizeVisibleAreaRunning && diff > 0) // If we are maximizing and the header was shown, make sure we hide it | 499 | if (maximizeVisibleAreaRunning && diff > 0) // If we are maximizing and the header was shown, make sure we hide it |
24 | 500 | m_headerItemShownHeight -= diff; | 500 | m_headerItemShownHeight -= diff; |
25 | @@ -981,7 +981,6 @@ | |||
26 | 981 | if (growUp) { | 981 | if (growUp) { |
27 | 982 | ListItem *firstItem = m_visibleItems.first(); | 982 | ListItem *firstItem = m_visibleItems.first(); |
28 | 983 | firstItem->setY(firstItem->y() - item->height()); | 983 | firstItem->setY(firstItem->y() - item->height()); |
29 | 984 | adjustMinYExtent(); | ||
30 | 985 | } | 984 | } |
31 | 986 | // Adding an item may break a "same section" chain, so check | 985 | // Adding an item may break a "same section" chain, so check |
32 | 987 | // if we need adding a new section item | 986 | // if we need adding a new section item |
33 | @@ -992,7 +991,6 @@ | |||
34 | 992 | if (growUp && nextItem->m_sectionItem) { | 991 | if (growUp && nextItem->m_sectionItem) { |
35 | 993 | ListItem *firstItem = m_visibleItems.first(); | 992 | ListItem *firstItem = m_visibleItems.first(); |
36 | 994 | firstItem->setY(firstItem->y() - nextItem->m_sectionItem->height()); | 993 | firstItem->setY(firstItem->y() - nextItem->m_sectionItem->height()); |
37 | 995 | adjustMinYExtent(); | ||
38 | 996 | } | 994 | } |
39 | 997 | } | 995 | } |
40 | 998 | } | 996 | } |
41 | @@ -1001,6 +999,7 @@ | |||
42 | 1001 | ListItem *firstItem = m_visibleItems.first(); | 999 | ListItem *firstItem = m_visibleItems.first(); |
43 | 1002 | firstItem->setY(oldFirstValidIndexPos); | 1000 | firstItem->setY(oldFirstValidIndexPos); |
44 | 1003 | } | 1001 | } |
45 | 1002 | adjustMinYExtent(); | ||
46 | 1004 | } else if (insert.index <= m_firstVisibleIndex) { | 1003 | } else if (insert.index <= m_firstVisibleIndex) { |
47 | 1005 | m_firstVisibleIndex += insert.count; | 1004 | m_firstVisibleIndex += insert.count; |
48 | 1006 | } | 1005 | } |
49 | 1007 | 1006 | ||
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 | 1589 | QCOMPARE(lvwph->m_headerItemShownHeight, 0.); | 1589 | QCOMPARE(lvwph->m_headerItemShownHeight, 0.); |
55 | 1590 | } | 1590 | } |
56 | 1591 | 1591 | ||
57 | 1592 | void testMaximizeVisibleAreaTopWithHalfPageHeaderUpDown() | ||
58 | 1593 | { | ||
59 | 1594 | testMaximizeVisibleAreaTopWithHalfPageHeader(); | ||
60 | 1595 | |||
61 | 1596 | changeContentY(-60); | ||
62 | 1597 | changeContentY(15); | ||
63 | 1598 | |||
64 | 1599 | QTRY_COMPARE(lvwph->m_visibleItems.count(), 4); | ||
65 | 1600 | QCOMPARE(lvwph->m_firstVisibleIndex, 0); | ||
66 | 1601 | verifyItem(0, -140., 150., false); | ||
67 | 1602 | verifyItem(1, 10., 200., false); | ||
68 | 1603 | verifyItem(2, 210, 350., false); | ||
69 | 1604 | verifyItem(3, 560, 350., true); | ||
70 | 1605 | QCOMPARE(lvwph->m_minYExtent, 0.); | ||
71 | 1606 | QCOMPARE(lvwph->m_clipItem->y(), 190.); | ||
72 | 1607 | QCOMPARE(lvwph->m_clipItem->clip(), true); | ||
73 | 1608 | QCOMPARE(lvwph->m_headerItem->y(), 140.); | ||
74 | 1609 | QCOMPARE(lvwph->m_headerItem->height(), 50.); | ||
75 | 1610 | QCOMPARE(lvwph->contentY(), 155.); | ||
76 | 1611 | QCOMPARE(lvwph->m_headerItemShownHeight, 35.); | ||
77 | 1612 | } | ||
78 | 1613 | |||
79 | 1592 | void testMaximizeVisibleAreaBottomWithHalfPageHeader() | 1614 | void testMaximizeVisibleAreaBottomWithHalfPageHeader() |
80 | 1593 | { | 1615 | { |
81 | 1594 | changeContentY(430); | 1616 | changeContentY(430); |
82 | 1595 | 1617 | ||
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 | 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")); |
88 | 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")); |
89 | 1736 | 1736 | ||
91 | 1737 | changeContentY(140); | 1737 | changeContentY(5); |
92 | 1738 | |||
93 | 1739 | QTRY_COMPARE(lvwph->m_visibleItems.count(), 5); | ||
94 | 1740 | QCOMPARE(lvwph->m_firstVisibleIndex, 0); | ||
95 | 1741 | verifyItem(0, 45., 90., false, "Agressive", false); | ||
96 | 1742 | verifyItem(1, 135., 50., false, QString(), false); | ||
97 | 1743 | verifyItem(2, 185., 150., false, QString(), false); | ||
98 | 1744 | verifyItem(3, 335., 240., false, "Regular", false); | ||
99 | 1745 | verifyItem(4, 575., 390., true, "Mild", true); | ||
100 | 1746 | QCOMPARE(lvwph->m_minYExtent, 0.); | ||
101 | 1747 | QCOMPARE(lvwph->m_clipItem->y(), 5.); | ||
102 | 1748 | QCOMPARE(lvwph->m_clipItem->clip(), false); | ||
103 | 1749 | QCOMPARE(lvwph->m_headerItem->y(), 0.); | ||
104 | 1750 | QCOMPARE(lvwph->m_headerItem->height(), 50.); | ||
105 | 1751 | QCOMPARE(lvwph->contentY(), 5.); | ||
106 | 1752 | QCOMPARE(lvwph->m_headerItemShownHeight, 0.); | ||
107 | 1753 | QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled); | ||
108 | 1754 | |||
109 | 1755 | changeContentY(135); | ||
110 | 1738 | 1756 | ||
111 | 1739 | QTRY_COMPARE(lvwph->m_visibleItems.count(), 5); | 1757 | QTRY_COMPARE(lvwph->m_visibleItems.count(), 5); |
112 | 1740 | QCOMPARE(lvwph->m_firstVisibleIndex, 0); | 1758 | QCOMPARE(lvwph->m_firstVisibleIndex, 0); |
113 | @@ -2076,6 +2094,68 @@ | |||
114 | 2076 | QCOMPARE(spy.count(), 0); | 2094 | QCOMPARE(spy.count(), 0); |
115 | 2077 | } | 2095 | } |
116 | 2078 | 2096 | ||
117 | 2097 | void testDownAndUp() | ||
118 | 2098 | { | ||
119 | 2099 | QMetaObject::invokeMethod(model, "removeItems", Q_ARG(QVariant, 1), Q_ARG(QVariant, 5)); | ||
120 | 2100 | |||
121 | 2101 | QTRY_COMPARE(lvwph->m_visibleItems.count(), 1); | ||
122 | 2102 | QCOMPARE(lvwph->m_firstVisibleIndex, 0); | ||
123 | 2103 | verifyItem(0, 50., 190., false, "Agressive", false); | ||
124 | 2104 | QCOMPARE(lvwph->m_minYExtent, 0.); | ||
125 | 2105 | QCOMPARE(lvwph->m_clipItem->y(), 0.); | ||
126 | 2106 | QCOMPARE(lvwph->m_clipItem->clip(), false); | ||
127 | 2107 | QCOMPARE(lvwph->m_headerItem->y(), 0.); | ||
128 | 2108 | QCOMPARE(lvwph->m_headerItem->height(), 50.); | ||
129 | 2109 | QCOMPARE(lvwph->contentY(), 0.); | ||
130 | 2110 | QCOMPARE(lvwph->m_headerItemShownHeight, 0.); | ||
131 | 2111 | QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled); | ||
132 | 2112 | |||
133 | 2113 | QTest::qWait(1); | ||
134 | 2114 | |||
135 | 2115 | changeContentY(-15); | ||
136 | 2116 | |||
137 | 2117 | QTRY_COMPARE(lvwph->m_visibleItems.count(), 1); | ||
138 | 2118 | QCOMPARE(lvwph->m_firstVisibleIndex, 0); | ||
139 | 2119 | verifyItem(0, 65., 190., false, "Agressive", false); | ||
140 | 2120 | QCOMPARE(lvwph->m_minYExtent, 0.); | ||
141 | 2121 | QCOMPARE(lvwph->m_clipItem->y(), -15.); | ||
142 | 2122 | QCOMPARE(lvwph->m_clipItem->clip(), false); | ||
143 | 2123 | QCOMPARE(lvwph->m_headerItem->y(), -15.); | ||
144 | 2124 | QCOMPARE(lvwph->m_headerItem->height(), 65.); | ||
145 | 2125 | QCOMPARE(lvwph->contentY(), -15.); | ||
146 | 2126 | QCOMPARE(lvwph->m_headerItemShownHeight, 0.); | ||
147 | 2127 | QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled); | ||
148 | 2128 | |||
149 | 2129 | changeContentY(25); | ||
150 | 2130 | |||
151 | 2131 | QTRY_COMPARE(lvwph->m_visibleItems.count(), 1); | ||
152 | 2132 | QCOMPARE(lvwph->m_firstVisibleIndex, 0); | ||
153 | 2133 | verifyItem(0, 40., 190., false, "Agressive", false); | ||
154 | 2134 | QCOMPARE(lvwph->m_minYExtent, 0.); | ||
155 | 2135 | QCOMPARE(lvwph->m_clipItem->y(), 10.); | ||
156 | 2136 | QCOMPARE(lvwph->m_clipItem->clip(), false); | ||
157 | 2137 | QCOMPARE(lvwph->m_headerItem->y(), 0.); | ||
158 | 2138 | QCOMPARE(lvwph->m_headerItem->height(), 50.); | ||
159 | 2139 | QCOMPARE(lvwph->contentY(), 10.); | ||
160 | 2140 | QCOMPARE(lvwph->m_headerItemShownHeight, 0.); | ||
161 | 2141 | QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled); | ||
162 | 2142 | |||
163 | 2143 | |||
164 | 2144 | QTest::mouseClick(view, Qt::LeftButton, Qt::NoModifier, QPoint(0, 0)); | ||
165 | 2145 | |||
166 | 2146 | QTRY_COMPARE(lvwph->m_visibleItems.count(), 1); | ||
167 | 2147 | QCOMPARE(lvwph->m_firstVisibleIndex, 0); | ||
168 | 2148 | verifyItem(0, 50., 190., false, "Agressive", false); | ||
169 | 2149 | QCOMPARE(lvwph->m_minYExtent, 0.); | ||
170 | 2150 | QCOMPARE(lvwph->m_clipItem->y(), 0.); | ||
171 | 2151 | QCOMPARE(lvwph->m_clipItem->clip(), false); | ||
172 | 2152 | QCOMPARE(lvwph->m_headerItem->y(), 0.); | ||
173 | 2153 | QCOMPARE(lvwph->m_headerItem->height(), 50.); | ||
174 | 2154 | QCOMPARE(lvwph->contentY(), 0.); | ||
175 | 2155 | QCOMPARE(lvwph->m_headerItemShownHeight, 0.); | ||
176 | 2156 | QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled); | ||
177 | 2157 | } | ||
178 | 2158 | |||
179 | 2079 | private: | 2159 | private: |
180 | 2080 | QQuickView *view; | 2160 | QQuickView *view; |
181 | 2081 | ListViewWithPageHeader *lvwph; | 2161 | ListViewWithPageHeader *lvwph; |
yes! fixes the issue for me!