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

Proposed by Albert Astals Cid
Status: Merged
Approved by: Michael Zanetti
Approved revision: 2058
Merged at revision: 2088
Proposed branch: lp:~aacid/unity8/sectionDelegateUpdates
Merge into: lp:unity8
Diff against target: 115 lines (+67/-8)
3 files modified
plugins/Dash/listviewwithpageheader.cpp (+35/-8)
plugins/Dash/listviewwithpageheader.h (+1/-0)
tests/plugins/Dash/listviewwithpageheadersectiontest.cpp (+31/-0)
To merge this branch: bzr merge lp:~aacid/unity8/sectionDelegateUpdates
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Michael Zanetti (community) Approve
Review via email: mp+278706@code.launchpad.net

Commit message

LVWPH: Process correctly section changes

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
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:2057
http://jenkins.qa.ubuntu.com/job/unity8-ci/6814/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5434
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/229/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1525
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/227/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1420
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1420
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/227
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/226
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4289
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5448
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5448/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25544
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-xenial-touch/75/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/228
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/228/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25545

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

review: Needs Fixing (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

Tested it, fixes the issue. Code looks good. There's one typo in a comment. Update that and I'll approve.

review: Needs Fixing
2058. By Albert Astals Cid

Typo--

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

Fixed the typo

Revision history for this message
Michael Zanetti (mzanetti) wrote :

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

yes

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

yes (on v+o)

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

yes

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

FAILED: Continuous integration, rev:2058
http://jenkins.qa.ubuntu.com/job/unity8-ci/6822/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-vivid-touch/5454
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-xenial-touch/237/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-vivid/1533
    FAILURE: http://jenkins.qa.ubuntu.com/job/unity8-qmluitest-xenial-amd64/235/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-amd64-ci/1428
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-vivid-i386-ci/1428
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-amd64-ci/235
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-xenial-i386-ci/234
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-vivid-touch/4304
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5468
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-vivid-armhf/5468/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25579
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-xenial-touch/81/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/236
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-xenial-armhf/236/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/25578

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

review: Needs Fixing (continuous-integration)
2059. By Albert Astals Cid

Merge

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 2015-11-20 15:01:39 +0000
3+++ plugins/Dash/listviewwithpageheader.cpp 2015-12-03 13:39:52 +0000
4@@ -768,6 +768,37 @@
5 return sectionItem;
6 }
7
8+void ListViewWithPageHeader::updateSectionItem(int modelIndex)
9+{
10+ ListItem *item = itemAtIndex(modelIndex);
11+ if (item) {
12+ const QString sectionText = m_delegateModel->stringValue(modelIndex, m_sectionProperty);
13+
14+ bool needSectionHeader = true;
15+ // if it is the same section as the previous item need to drop the section
16+ if (modelIndex > 0) {
17+ const QString prevSection = m_delegateModel->stringValue(modelIndex - 1, m_sectionProperty);
18+ if (sectionText == prevSection) {
19+ needSectionHeader = false;
20+ }
21+ }
22+
23+ if (needSectionHeader) {
24+ if (!item->sectionItem()) {
25+ item->setSectionItem(getSectionItem(sectionText));
26+ } else {
27+ QQmlContext *context = QQmlEngine::contextForObject(item->sectionItem())->parentContext();
28+ context->setContextProperty(QStringLiteral("section"), sectionText);
29+ }
30+ } else {
31+ if (item->sectionItem()) {
32+ item->sectionItem()->deleteLater();
33+ item->setSectionItem(nullptr);
34+ }
35+ }
36+ }
37+}
38+
39 bool ListViewWithPageHeader::removeNonVisibleItems(qreal bufferFrom, qreal bufferTo)
40 {
41 // qDebug() << "ListViewWithPageHeader::removeNonVisibleItems" << bufferFrom << bufferTo;
42@@ -1053,15 +1084,11 @@
43 }
44
45 Q_FOREACH(const QQmlChangeSet::Change change, changeSet.changes()) {
46-// qDebug() << "ListViewWithPageHeader::onModelUpdated Change" << change.index << change.count;
47- for (int i = change.index; i < change.count; ++i) {
48- ListItem *item = itemAtIndex(i);
49- if (item && item->sectionItem()) {
50- QQmlContext *context = QQmlEngine::contextForObject(item->sectionItem())->parentContext();
51- const QString sectionText = m_delegateModel->stringValue(i, m_sectionProperty);
52- context->setContextProperty(QStringLiteral("section"), sectionText);
53- }
54+ for (int i = change.start(); i < change.end(); ++i) {
55+ updateSectionItem(i);
56 }
57+ // Also update the section header for the next item after the change since it may be influenced
58+ updateSectionItem(change.end());
59 }
60
61 if (m_firstVisibleIndex != oldFirstVisibleIndex) {
62
63=== modified file 'plugins/Dash/listviewwithpageheader.h'
64--- plugins/Dash/listviewwithpageheader.h 2014-12-03 10:28:28 +0000
65+++ plugins/Dash/listviewwithpageheader.h 2015-12-03 13:39:52 +0000
66@@ -167,6 +167,7 @@
67 void updateWatchedRoles();
68 QQuickItem *getSectionItem(int modelIndex, bool alreadyInserted);
69 QQuickItem *getSectionItem(const QString &sectionText);
70+ void updateSectionItem(int modelIndex);
71
72 QQmlDelegateModel *m_delegateModel;
73
74
75=== modified file 'tests/plugins/Dash/listviewwithpageheadersectiontest.cpp'
76--- tests/plugins/Dash/listviewwithpageheadersectiontest.cpp 2015-09-02 08:04:41 +0000
77+++ tests/plugins/Dash/listviewwithpageheadersectiontest.cpp 2015-12-03 13:39:52 +0000
78@@ -2149,6 +2149,37 @@
79 QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
80 }
81
82+ void changeSectionProperty()
83+ {
84+ model->setProperty(1, "type", "Rojo");
85+ verifyItem(1, 240., 240., false, "Rojo", false);
86+
87+ model->setProperty(1, "type", "Agressive");
88+ verifyItem(1, 240., 200., false, QString(), true);
89+
90+ model->setProperty(1, "type", "Rojo");
91+ verifyItem(1, 240., 240., false, "Rojo", false);
92+
93+ model->setProperty(1, "type", "Mild");
94+ verifyItem(1, 240., 240., false, "Mild", false);
95+ verifyItem(2, 480., 350., false, QString(), false);
96+
97+ model->setProperty(1, "type", "Rojo");
98+ verifyItem(1, 240., 240., false, "Rojo", false);
99+ verifyItem(2, 480., 390., false, "Mild", false);
100+
101+ model->setProperty(2, "type", "Agressive");
102+ verifyItem(2, 480., 390., false, "Agressive", false);
103+
104+ model->setProperty(1, "type", "Agressive");
105+ verifyItem(1, 240., 200., false, QString(), true);
106+ verifyItem(2, 440., 350., false, QString(), false);
107+
108+ model->setProperty(1, "type", "Rojo");
109+ verifyItem(1, 240., 240., false, "Rojo", false);
110+ verifyItem(2, 480., 390., false, "Agressive", false);
111+ }
112+
113 private:
114 QQuickView *view;
115 ListViewWithPageHeader *lvwph;

Subscribers

People subscribed via source and target branches