Unity 8

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

  1. headerless_category_margin
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Michał Sawicz
Approved revision: 944
Merged at revision: 954
Proposed branch: lp:~aacid/unity8/headerless_category_margin
Merge into: lp:unity8
Diff against target: 290 lines (+58/-36)
5 files modified
plugins/Dash/listviewwithpageheader.cpp (+39/-28)
plugins/Dash/listviewwithpageheader.h (+4/-0)
qml/Dash/GenericScopeView.qml (+1/-0)
tests/plugins/Dash/listviewwithpageheadersectionexternalmodeltest.cpp (+3/-3)
tests/plugins/Dash/listviewwithpageheadersectiontest.cpp (+11/-5)
To merge this branch: bzr merge lp:~aacid/unity8/headerless_category_margin
Reviewer Review Type Date Requested Status
Paweł Stołowski (community) Approve
Michał Sawicz Approve
PS Jenkins bot (community) continuous-integration Needs Fixing
Review via email: mp+222132@code.launchpad.net

Commit message

Improvements for headerless categories

LVPWH: No section name -> no header
LVPWH: New hasSectionHeader context property for delegates
GSV: Add topMargin if no hasSectionHeader

Description of the change

* Are there any related MPs required for this MP to build/function as expected?
No

* Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

* Did you make sure that your branch does not contain spurious tags?
Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

* If you changed the UI, has there been a design review?
N/A

To post a comment you must log in.
Revision history for this message
Albert Astals Cid (aacid) wrote :

It looks bigger than it is, since i made m_sectionItem private and replaced its uses with getter+setter

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

FAILED: Continuous integration, rev:944
http://jenkins.qa.ubuntu.com/job/unity8-ci/3097/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/681
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1835/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-amd64-ci/191
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/191
        deb: http://jenkins.qa.ubuntu.com/job/unity8-utopic-armhf-ci/191/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-utopic-i386-ci/191
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/1112
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1293
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1293/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/8105

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?

Yup!

 * Did CI run pass? If not, please explain why.

Don't ask... Complete havoc happened via a sync from debian.

review: Approve
Revision history for this message
Paweł Stołowski (stolowski) wrote :

Thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/Dash/listviewwithpageheader.cpp'
2--- plugins/Dash/listviewwithpageheader.cpp 2014-04-24 14:13:21 +0000
3+++ plugins/Dash/listviewwithpageheader.cpp 2014-06-05 08:44:16 +0000
4@@ -138,6 +138,13 @@
5 QQuickItemPrivate::get(m_sectionItem)->setCulled(culled);
6 }
7
8+void ListViewWithPageHeader::ListItem::setSectionItem(QQuickItem *sectionItem)
9+{
10+ m_sectionItem = sectionItem;
11+ QQmlContext *context = QQmlEngine::contextForObject(m_item)->parentContext();
12+ context->setContextProperty(QLatin1String("hasSectionHeader"), QVariant::fromValue<bool>(sectionItem != nullptr));
13+}
14+
15 ListViewWithPageHeader::ListViewWithPageHeader()
16 : m_delegateModel(nullptr)
17 , m_asyncRequestedIndex(-1)
18@@ -422,7 +429,7 @@
19
20 ListItem *listItem = itemAtIndex(modelIndex);
21 if (listItem) {
22- return maximizeVisibleArea(listItem, itemHeight + (listItem->m_sectionItem ? listItem->m_sectionItem->height() : 0));
23+ return maximizeVisibleArea(listItem, itemHeight + (listItem->sectionItem() ? listItem->sectionItem()->height() : 0));
24 }
25
26 return false;
27@@ -440,7 +447,7 @@
28 contentYAnimationType = ContentYAnimationMaximizeVisibleArea;
29 m_contentYAnimation->start();
30 } else if ((listItemY < contentY() && listItemY + listItemHeight < contentY() + height()) ||
31- (m_topSectionItem && !listItem->m_sectionItem && listItemY - m_topSectionItem->height() < contentY() && listItemY + listItemHeight < contentY() + height()))
32+ (m_topSectionItem && !listItem->sectionItem() && listItemY - m_topSectionItem->height() < contentY() && listItemY + listItemHeight < contentY() + height()))
33 {
34 // we can scroll the list down to show more stuff
35 auto realVisibleListItemY = listItemY;
36@@ -449,7 +456,7 @@
37 // item we have to make sure to scroll it a bit more so that it is not underlapping
38 // the top section sticky item
39 bool topSectionShown = !QQuickItemPrivate::get(m_topSectionItem)->culled;
40- if (topSectionShown && !listItem->m_sectionItem) {
41+ if (topSectionShown && !listItem->sectionItem()) {
42 realVisibleListItemY -= m_topSectionItem->height();
43 }
44 }
45@@ -645,7 +652,7 @@
46 if (flags & QQmlDelegateModel::Destroyed) {
47 item->setParentItem(nullptr);
48 }
49- delete listItem->m_sectionItem;
50+ delete listItem->sectionItem();
51 delete listItem;
52 }
53
54@@ -672,6 +679,9 @@
55 return nullptr;
56
57 const QString section = m_delegateModel->stringValue(modelIndex, m_sectionProperty);
58+ if (section.isEmpty())
59+ return nullptr;
60+
61 if (modelIndex > 0) {
62 const QString prevSection = m_delegateModel->stringValue(modelIndex - 1, m_sectionProperty);
63 if (section == prevSection)
64@@ -684,8 +694,8 @@
65 // Steal the section header
66 ListItem *nextItem = itemAtIndex(modelIndex); // Not +1 since not yet inserted into m_visibleItems
67 if (nextItem) {
68- QQuickItem *sectionItem = nextItem->m_sectionItem;
69- nextItem->m_sectionItem = nullptr;
70+ QQuickItem *sectionItem = nextItem->sectionItem();
71+ nextItem->setSectionItem(nullptr);
72 return sectionItem;
73 }
74 }
75@@ -793,7 +803,7 @@
76 // qDebug() << "ListViewWithPageHeader::createItem::We have the item" << modelIndex << item;
77 ListItem *listItem = new ListItem;
78 listItem->m_item = item;
79- listItem->m_sectionItem = getSectionItem(modelIndex, false /*Not yet inserted into m_visibleItems*/);
80+ listItem->setSectionItem(getSectionItem(modelIndex, false /*Not yet inserted into m_visibleItems*/));
81 QQuickItemPrivate::get(item)->addItemChangeListener(this, QQuickItemPrivate::Geometry);
82 ListItem *prevItem = itemAtIndex(modelIndex - 1);
83 bool lostItem = false; // Is an item that we requested async but because of model changes
84@@ -832,8 +842,8 @@
85 m_firstVisibleIndex = modelIndex;
86 polish();
87 }
88- if (listItem->m_sectionItem) {
89- QQmlContext *context = QQmlEngine::contextForObject(listItem->m_sectionItem)->parentContext();
90+ if (listItem->sectionItem()) {
91+ QQmlContext *context = QQmlEngine::contextForObject(listItem->sectionItem())->parentContext();
92 context->setContextProperty(QLatin1String("delegateIndex"), modelIndex);
93 }
94 adjustMinYExtent();
95@@ -861,6 +871,7 @@
96 QQmlContext *context = QQmlEngine::contextForObject(item)->parentContext();
97 context->setContextProperty(QLatin1String("ListViewWithPageHeader"), this);
98 context->setContextProperty(QLatin1String("heightToClip"), QVariant::fromValue<int>(0));
99+ context->setContextProperty(QLatin1String("hasSectionHeader"), QVariant::fromValue<bool>(false));
100 if (modelIndex == m_asyncRequestedIndex) {
101 createItem(modelIndex, false);
102 refill();
103@@ -915,11 +926,11 @@
104 if (visibleIndex >= 0 && visibleIndex < m_visibleItems.count()) {
105 ListItem *item = m_visibleItems[visibleIndex];
106 // Pass the section item down if needed
107- if (item->m_sectionItem && visibleIndex + 1 < m_visibleItems.count()) {
108+ if (item->sectionItem() && visibleIndex + 1 < m_visibleItems.count()) {
109 ListItem *nextItem = m_visibleItems[visibleIndex + 1];
110- if (!nextItem->m_sectionItem) {
111- nextItem->m_sectionItem = item->m_sectionItem;
112- item->m_sectionItem = nullptr;
113+ if (!nextItem->sectionItem()) {
114+ nextItem->setSectionItem(item->sectionItem());
115+ item->setSectionItem(nullptr);
116 }
117 }
118 releaseItem(item);
119@@ -982,11 +993,11 @@
120 // if we need adding a new section item
121 if (m_sectionDelegate) {
122 ListItem *nextItem = itemAtIndex(modelIndex + 1);
123- if (nextItem && !nextItem->m_sectionItem) {
124- nextItem->m_sectionItem = getSectionItem(modelIndex + 1, true /* alredy inserted into m_visibleItems*/);
125- if (growUp && nextItem->m_sectionItem) {
126+ if (nextItem && !nextItem->sectionItem()) {
127+ nextItem->setSectionItem(getSectionItem(modelIndex + 1, true /* alredy inserted into m_visibleItems*/));
128+ if (growUp && nextItem->sectionItem()) {
129 ListItem *firstItem = m_visibleItems.first();
130- firstItem->setY(firstItem->y() - nextItem->m_sectionItem->height());
131+ firstItem->setY(firstItem->y() - nextItem->sectionItem()->height());
132 }
133 }
134 }
135@@ -1012,8 +1023,8 @@
136 // qDebug() << "ListViewWithPageHeader::onModelUpdated Change" << change.index << change.count;
137 for (int i = change.index; i < change.count; ++i) {
138 ListItem *item = itemAtIndex(i);
139- if (item && item->m_sectionItem) {
140- QQmlContext *context = QQmlEngine::contextForObject(item->m_sectionItem)->parentContext();
141+ if (item && item->sectionItem()) {
142+ QQmlContext *context = QQmlEngine::contextForObject(item->sectionItem())->parentContext();
143 const QString sectionText = m_delegateModel->stringValue(i, m_sectionProperty);
144 context->setContextProperty(QLatin1String("section"), sectionText);
145 }
146@@ -1026,8 +1037,8 @@
147
148 for (int i = 0; i < m_visibleItems.count(); ++i) {
149 ListItem *item = m_visibleItems[i];
150- if (item->m_sectionItem) {
151- QQmlContext *context = QQmlEngine::contextForObject(item->m_sectionItem)->parentContext();
152+ if (item->sectionItem()) {
153+ QQmlContext *context = QQmlEngine::contextForObject(item->sectionItem())->parentContext();
154 context->setContextProperty(QLatin1String("delegateIndex"), m_firstVisibleIndex + i);
155 }
156 }
157@@ -1156,18 +1167,18 @@
158 if (topSectionStickPos > pos) {
159 showStickySectionItem = true;
160 } else if (topSectionStickPos == pos) {
161- showStickySectionItem = !item->m_sectionItem;
162+ showStickySectionItem = !item->sectionItem();
163 } else {
164 showStickySectionItem = false;
165 }
166 if (!showStickySectionItem) {
167 QQuickItemPrivate::get(m_topSectionItem)->setCulled(true);
168- if (item->m_sectionItem) {
169+ if (item->sectionItem()) {
170 // This seems it should happen since why would we cull the top section
171 // if the first visible item has no section header? This only happens briefly
172 // when increasing the height of a list that is at the bottom, the m_topSectionItem
173 // gets shown shortly in the next polish call
174- QQuickItemPrivate::get(item->m_sectionItem)->setCulled(false);
175+ QQuickItemPrivate::get(item->sectionItem())->setCulled(false);
176 }
177 } else {
178 // Update the top sticky section header
179@@ -1186,14 +1197,14 @@
180 delegateIndex--;
181 }
182 context->setContextProperty(QLatin1String("delegateIndex"), delegateIndex);
183- if (item->m_sectionItem) {
184- QQuickItemPrivate::get(item->m_sectionItem)->setCulled(true);
185+ if (item->sectionItem()) {
186+ QQuickItemPrivate::get(item->sectionItem())->setCulled(true);
187 }
188 }
189 }
190 }
191 QQmlContext *context = QQmlEngine::contextForObject(item->m_item)->parentContext();
192- const qreal clipFrom = visibleFrom + (!item->m_sectionItem && m_topSectionItem && !QQuickItemPrivate::get(m_topSectionItem)->culled ? m_topSectionItem->height() : 0);
193+ const qreal clipFrom = visibleFrom + (!item->sectionItem() && m_topSectionItem && !QQuickItemPrivate::get(m_topSectionItem)->culled ? m_topSectionItem->height() : 0);
194 if (!cull && pos < clipFrom) {
195 context->setContextProperty(QLatin1String("heightToClip"), clipFrom - pos);
196 } else {
197@@ -1210,7 +1221,7 @@
198 if (firstReallyVisibleItem >= 0) {
199 for (int i = firstReallyVisibleItem - m_firstVisibleIndex + 1; i < m_visibleItems.count(); ++i) {
200 ListItem *item = m_visibleItems[i];
201- if (item->m_sectionItem) {
202+ if (item->sectionItem()) {
203 if (m_topSectionItem->y() + m_topSectionItem->height() > item->y()) {
204 m_topSectionItem->setY(item->y() - m_topSectionItem->height());
205 }
206
207=== modified file 'plugins/Dash/listviewwithpageheader.h'
208--- plugins/Dash/listviewwithpageheader.h 2014-04-24 14:13:21 +0000
209+++ plugins/Dash/listviewwithpageheader.h 2014-06-05 08:44:16 +0000
210@@ -128,7 +128,11 @@
211 bool culled() const;
212 void setCulled(bool culled);
213
214+ QQuickItem *sectionItem() const { return m_sectionItem; }
215+ void setSectionItem(QQuickItem *sectionItem);
216+
217 QQuickItem *m_item;
218+ private:
219 QQuickItem *m_sectionItem;
220 };
221
222
223=== modified file 'qml/Dash/GenericScopeView.qml'
224--- qml/Dash/GenericScopeView.qml 2014-05-28 07:17:08 +0000
225+++ qml/Dash/GenericScopeView.qml 2014-06-05 08:44:16 +0000
226@@ -141,6 +141,7 @@
227 top: parent.top
228 left: parent.left
229 right: parent.right
230+ topMargin: hasSectionHeader ? 0 : units.gu(2)
231 }
232
233 source: {
234
235=== modified file 'tests/plugins/Dash/listviewwithpageheadersectionexternalmodeltest.cpp'
236--- tests/plugins/Dash/listviewwithpageheadersectionexternalmodeltest.cpp 2014-05-05 22:07:57 +0000
237+++ tests/plugins/Dash/listviewwithpageheadersectionexternalmodeltest.cpp 2014-06-05 08:44:16 +0000
238@@ -82,10 +82,10 @@
239 QTRY_COMPARE(lvwph->m_visibleItems[visibleIndex]->y(), pos);
240 QTRY_COMPARE(lvwph->m_visibleItems[visibleIndex]->height(), height);
241 QCOMPARE(QQuickItemPrivate::get(lvwph->m_visibleItems[visibleIndex]->m_item)->culled, culled);
242- QCOMPARE(section(lvwph->m_visibleItems[visibleIndex]->m_sectionItem), sectionHeader);
243+ QCOMPARE(section(lvwph->m_visibleItems[visibleIndex]->sectionItem()), sectionHeader);
244 if (!sectionHeader.isNull()) {
245- QCOMPARE(QQuickItemPrivate::get(lvwph->m_visibleItems[visibleIndex]->m_sectionItem)->culled, sectionHeaderCulled);
246- QCOMPARE(sectionDelegateIndex(lvwph->m_visibleItems[visibleIndex]->m_sectionItem), lvwph->m_firstVisibleIndex + visibleIndex);
247+ QCOMPARE(QQuickItemPrivate::get(lvwph->m_visibleItems[visibleIndex]->sectionItem())->culled, sectionHeaderCulled);
248+ QCOMPARE(sectionDelegateIndex(lvwph->m_visibleItems[visibleIndex]->sectionItem()), lvwph->m_firstVisibleIndex + visibleIndex);
249 }
250 }
251
252
253=== modified file 'tests/plugins/Dash/listviewwithpageheadersectiontest.cpp'
254--- tests/plugins/Dash/listviewwithpageheadersectiontest.cpp 2014-05-05 22:07:57 +0000
255+++ tests/plugins/Dash/listviewwithpageheadersectiontest.cpp 2014-06-05 08:44:16 +0000
256@@ -39,10 +39,11 @@
257 QTRY_COMPARE(lvwph->m_visibleItems[visibleIndex]->y(), pos);
258 QTRY_COMPARE(lvwph->m_visibleItems[visibleIndex]->height(), height);
259 QCOMPARE(QQuickItemPrivate::get(lvwph->m_visibleItems[visibleIndex]->m_item)->culled, culled);
260- QCOMPARE(section(lvwph->m_visibleItems[visibleIndex]->m_sectionItem), sectionHeader);
261+ QCOMPARE(section(lvwph->m_visibleItems[visibleIndex]->sectionItem()), sectionHeader);
262+ QTRY_COMPARE(hasSectionHeaderProperty(lvwph->m_visibleItems[visibleIndex]->m_item), !sectionHeader.isNull());
263 if (!sectionHeader.isNull()) {
264- QCOMPARE(QQuickItemPrivate::get(lvwph->m_visibleItems[visibleIndex]->m_sectionItem)->culled, sectionHeaderCulled);
265- QCOMPARE(sectionDelegateIndex(lvwph->m_visibleItems[visibleIndex]->m_sectionItem), lvwph->m_firstVisibleIndex + visibleIndex);
266+ QCOMPARE(QQuickItemPrivate::get(lvwph->m_visibleItems[visibleIndex]->sectionItem())->culled, sectionHeaderCulled);
267+ QCOMPARE(sectionDelegateIndex(lvwph->m_visibleItems[visibleIndex]->sectionItem()), lvwph->m_firstVisibleIndex + visibleIndex);
268 }
269 }
270
271@@ -109,12 +110,17 @@
272 QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
273 }
274
275- QString section(QQuickItem *item)
276+ bool hasSectionHeaderProperty(QQuickItem *item) const
277+ {
278+ return item ? QQmlEngine::contextForObject(item)->parentContext()->contextProperty(QLatin1String("hasSectionHeader")).toBool() : false;
279+ }
280+
281+ QString section(QQuickItem *item) const
282 {
283 return item ? QQmlEngine::contextForObject(item)->parentContext()->contextProperty(QLatin1String("section")).toString() : QString();
284 }
285
286- int sectionDelegateIndex(QQuickItem *item)
287+ int sectionDelegateIndex(QQuickItem *item) const
288 {
289 return item ? QQmlEngine::contextForObject(item)->parentContext()->contextProperty(QLatin1String("delegateIndex")).toInt() : -1;
290 }

Subscribers

People subscribed via source and target branches