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

[Albert Astals Cid]

LVWPH: Process correctly section changes

 * 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

[PS Jenkins bot (ps-jenkins)]

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)

[Michael Zanetti (mzanetti)]

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

review: Needs Fixing

[lp:~aacid/unity8/sectionDelegateUpdates]

2058. By Albert Astals Cid on 2015-11-27

Typo--

[Albert Astals Cid (aacid)]

Fixed the typo

[Michael Zanetti (mzanetti)]

 * 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

[PS Jenkins bot (ps-jenkins)]

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)

[lp:~aacid/unity8/sectionDelegateUpdates]

2059. By Albert Astals Cid on 2015-12-03

Merge

=== modified file 'plugins/Dash/listviewwithpageheader.cpp'
--- plugins/Dash/listviewwithpageheader.cpp	2015-11-20 15:01:39 +0000
+++ plugins/Dash/listviewwithpageheader.cpp	2015-12-03 13:39:52 +0000
@@ -768,6 +768,37 @@
     return sectionItem;
 }
 
+void ListViewWithPageHeader::updateSectionItem(int modelIndex)
+{
+    ListItem *item = itemAtIndex(modelIndex);
+    if (item) {
+        const QString sectionText = m_delegateModel->stringValue(modelIndex, m_sectionProperty);
+
+        bool needSectionHeader = true;
+        // if it is the same section as the previous item need to drop the section
+        if (modelIndex > 0) {
+            const QString prevSection = m_delegateModel->stringValue(modelIndex - 1, m_sectionProperty);
+            if (sectionText == prevSection) {
+                needSectionHeader = false;
+            }
+        }
+
+        if (needSectionHeader) {
+            if (!item->sectionItem()) {
+                item->setSectionItem(getSectionItem(sectionText));
+            } else {
+                QQmlContext *context = QQmlEngine::contextForObject(item->sectionItem())->parentContext();
+                context->setContextProperty(QStringLiteral("section"), sectionText);
+            }
+        } else {
+            if (item->sectionItem()) {
+                item->sectionItem()->deleteLater();
+                item->setSectionItem(nullptr);
+            }
+        }
+    }
+}
+
 bool ListViewWithPageHeader::removeNonVisibleItems(qreal bufferFrom, qreal bufferTo)
 {
 //     qDebug() << "ListViewWithPageHeader::removeNonVisibleItems" << bufferFrom << bufferTo;
@@ -1053,15 +1084,11 @@
     }
 
     Q_FOREACH(const QQmlChangeSet::Change change, changeSet.changes()) {
-//         qDebug() << "ListViewWithPageHeader::onModelUpdated Change" << change.index << change.count;
-        for (int i = change.index; i < change.count; ++i) {
-            ListItem *item = itemAtIndex(i);
-            if (item && item->sectionItem()) {
-                QQmlContext *context = QQmlEngine::contextForObject(item->sectionItem())->parentContext();
-                const QString sectionText = m_delegateModel->stringValue(i, m_sectionProperty);
-                context->setContextProperty(QStringLiteral("section"), sectionText);
-            }
+        for (int i = change.start(); i < change.end(); ++i) {
+            updateSectionItem(i);
         }
+        // Also update the section header for the next item after the change since it may be influenced
+        updateSectionItem(change.end());
     }
 
     if (m_firstVisibleIndex != oldFirstVisibleIndex) {

=== modified file 'plugins/Dash/listviewwithpageheader.h'
--- plugins/Dash/listviewwithpageheader.h	2014-12-03 10:28:28 +0000
+++ plugins/Dash/listviewwithpageheader.h	2015-12-03 13:39:52 +0000
@@ -167,6 +167,7 @@
     void updateWatchedRoles();
     QQuickItem *getSectionItem(int modelIndex, bool alreadyInserted);
     QQuickItem *getSectionItem(const QString &sectionText);
+    void updateSectionItem(int modelIndex);
 
     QQmlDelegateModel *m_delegateModel;
 

=== modified file 'tests/plugins/Dash/listviewwithpageheadersectiontest.cpp'
--- tests/plugins/Dash/listviewwithpageheadersectiontest.cpp	2015-09-02 08:04:41 +0000
+++ tests/plugins/Dash/listviewwithpageheadersectiontest.cpp	2015-12-03 13:39:52 +0000
@@ -2149,6 +2149,37 @@
         QVERIFY(QQuickItemPrivate::get(lvwph->m_topSectionItem)->culled);
     }
 
+    void changeSectionProperty()
+    {
+        model->setProperty(1, "type", "Rojo");
+        verifyItem(1, 240., 240., false, "Rojo", false);
+
+        model->setProperty(1, "type", "Agressive");
+        verifyItem(1, 240., 200., false, QString(), true);
+
+        model->setProperty(1, "type", "Rojo");
+        verifyItem(1, 240., 240., false, "Rojo", false);
+
+        model->setProperty(1, "type", "Mild");
+        verifyItem(1, 240., 240., false, "Mild", false);
+        verifyItem(2, 480., 350., false, QString(), false);
+
+        model->setProperty(1, "type", "Rojo");
+        verifyItem(1, 240., 240., false, "Rojo", false);
+        verifyItem(2, 480., 390., false, "Mild", false);
+
+        model->setProperty(2, "type", "Agressive");
+        verifyItem(2, 480., 390., false, "Agressive", false);
+
+        model->setProperty(1, "type", "Agressive");
+        verifyItem(1, 240., 200., false, QString(), true);
+        verifyItem(2, 440., 350., false, QString(), false);
+
+        model->setProperty(1, "type", "Rojo");
+        verifyItem(1, 240., 240., false, "Rojo", false);
+        verifyItem(2, 480., 390., false, "Agressive", false);
+    }
+
 private:
     QQuickView *view;
     ListViewWithPageHeader *lvwph;