Unity 8

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

  1. verticalJournalBug1599754
  2. Merge into trunk
Proposed by Albert Astals Cid on 2016-07-12
Status: Merged
Approved by: Michał Sawicz on 2016-07-13
Approved revision: 2530
Merged at revision: 2566
Proposed branch: lp:~aacid/unity8/verticalJournalBug1599754
Merge into: lp:unity8
Diff against target: 179 lines (+89/-2)
5 files modified
plugins/Dash/abstractdashview.cpp (+13/-0)
plugins/Dash/abstractdashview.h (+2/-1)
plugins/Dash/verticaljournal.cpp (+9/-1)
plugins/Dash/verticaljournal.h (+3/-0)
tests/plugins/Dash/verticaljournaltest.cpp (+62/-0)
To merge this branch: bzr merge lp:~aacid/unity8/verticalJournalBug1599754
Reviewer Review Type Date Requested Status
Unity8 CI Bot continuous-integration Needs Fixing on 2016-07-13
Michał Sawicz 2016-07-12 Approve on 2016-07-13
Review via email: mp+299818@code.launchpad.net

Commit Message

VerticalJournal improvements regarding model insertions and item height changes

insertions in the middle need to trigger a reset
Item height changes need to trigger a relayout

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

 * 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.
lp:~aacid/unity8/verticalJournalBug1599754 updated on 2016-07-12
2528. By Albert Astals Cid on 2016-07-12

VerticalJournal improvements regarding model insertions and item height changes

insertions in the middle need to trigger a reset
Item height changes need to trigger a relayout

2529. By Albert Astals Cid on 2016-07-12

eof

2530. By Albert Astals Cid on 2016-07-12

typo fix

Michał Sawicz (saviq) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Y
 * Did CI run pass? If not, please explain why.
Waiting

review: Approve
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:2530
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1708/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2258
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1215
    FAILURE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1215/console
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1215
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2286
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2195
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2195
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2195
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2186
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2186/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2186
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2186/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2186
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2186/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2186
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2186/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2186
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2186/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2186
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2186/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2186
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2186/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2186
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2186/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2186
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2186/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1708/rebuild

review: Needs Fixing (continuous-integration)
Michał Sawicz (saviq) wrote :

Unrelated failures in CI ;/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/Dash/abstractdashview.cpp'
2--- plugins/Dash/abstractdashview.cpp 2015-08-19 13:56:21 +0000
3+++ plugins/Dash/abstractdashview.cpp 2016-07-12 15:08:25 +0000
4@@ -16,6 +16,8 @@
5
6 #include "abstractdashview.h"
7
8+#include <private/qquickitem_p.h>
9+
10 AbstractDashView::AbstractDashView()
11 : m_delegateModel(nullptr)
12 , m_asyncRequestedIndex(-1)
13@@ -249,6 +251,7 @@
14 }
15 return nullptr;
16 } else {
17+ QQuickItemPrivate::get(item)->addItemChangeListener(this, QQuickItemPrivate::Geometry);
18 addItemToView(modelIndex, item);
19 return item;
20 }
21@@ -256,6 +259,7 @@
22
23 void AbstractDashView::releaseItem(QQuickItem *item)
24 {
25+ QQuickItemPrivate::get(item)->removeItemChangeListener(this, QQuickItemPrivate::Geometry);
26 QQmlDelegateModel::ReleaseFlags flags = m_delegateModel->release(item);
27 if (flags & QQmlDelegateModel::Destroyed) {
28 item->setParentItem(nullptr);
29@@ -294,6 +298,15 @@
30 cleanupExistingItems();
31 } else {
32 processModelRemoves(changeSet.removes());
33+
34+ // The current AbstractDashViews do not support insertions that are not at the end
35+ // so reset if that happens
36+ Q_FOREACH(const QQmlChangeSet::Change insert, changeSet.inserts()) {
37+ if (insert.index < m_delegateModel->count() - 1) {
38+ cleanupExistingItems();
39+ break;
40+ }
41+ }
42 }
43 polish();
44 }
45
46=== modified file 'plugins/Dash/abstractdashview.h'
47--- plugins/Dash/abstractdashview.h 2016-06-02 09:32:33 +0000
48+++ plugins/Dash/abstractdashview.h 2016-07-12 15:08:25 +0000
49@@ -23,9 +23,10 @@
50 class QQmlComponent;
51
52 #include <private/qqmldelegatemodel_p.h>
53+#include <private/qquickitemchangelistener_p.h>
54 #include <qqmlinfo.h>
55
56-class AbstractDashView : public QQuickItem
57+class AbstractDashView : public QQuickItem, public QQuickItemChangeListener
58 {
59 Q_OBJECT
60
61
62=== modified file 'plugins/Dash/verticaljournal.cpp'
63--- plugins/Dash/verticaljournal.cpp 2016-06-02 09:32:33 +0000
64+++ plugins/Dash/verticaljournal.cpp 2016-07-12 15:08:25 +0000
65@@ -94,7 +94,7 @@
66
67 if (*modelIndex > 0) {
68 Q_ASSERT(m_indexColumnMap.contains(*modelIndex));
69- while (m_indexColumnMap[*modelIndex] != columnToAddTo) {
70+ while (*modelIndex > 0 && m_indexColumnMap[*modelIndex] != columnToAddTo) {
71 // We found out that we have to add to columnToAddTo
72 // and thought that we had to add *modelIndex, but history tells
73 // it is not correct, so find up from *modelIndex until we found the index
74@@ -268,3 +268,11 @@
75 }
76 }
77 }
78+
79+void VerticalJournal::itemGeometryChanged(QQuickItem * /*item*/, const QRectF &newGeometry, const QRectF &oldGeometry)
80+{
81+ const qreal heightDiff = newGeometry.height() - oldGeometry.height();
82+ if (heightDiff != 0) {
83+ relayout();
84+ }
85+}
86
87=== modified file 'plugins/Dash/verticaljournal.h'
88--- plugins/Dash/verticaljournal.h 2015-11-20 15:01:39 +0000
89+++ plugins/Dash/verticaljournal.h 2016-07-12 15:08:25 +0000
90@@ -65,6 +65,9 @@
91 qreal columnWidth() const;
92 void setColumnWidth(qreal columnWidth);
93
94+protected:
95+ void itemGeometryChanged(QQuickItem *item, const QRectF &newGeometry, const QRectF &oldGeometry) override;
96+
97 Q_SIGNALS:
98 void columnWidthChanged();
99
100
101=== modified file 'tests/plugins/Dash/verticaljournaltest.cpp'
102--- tests/plugins/Dash/verticaljournaltest.cpp 2016-06-27 18:44:41 +0000
103+++ tests/plugins/Dash/verticaljournaltest.cpp 2016-07-12 15:08:25 +0000
104@@ -72,6 +72,20 @@
105 endRemoveRows();
106 }
107
108+ void insertString(int index, const QString& string)
109+ {
110+ beginInsertRows(QModelIndex(), index, index);
111+ m_list.insert(index, string);
112+ endInsertRows();
113+ }
114+
115+ void changeString(int i, const QString& string)
116+ {
117+ m_list[i] = string;
118+ auto idx = index(i, 0);
119+ dataChanged(idx, idx);
120+ }
121+
122 private:
123 QStringList m_list;
124 };
125@@ -488,6 +502,54 @@
126 QTRY_COMPARE(vj->implicitHeight(), 0.);
127 }
128
129+ void testInsertItem()
130+ {
131+ // Remove a few items from the end so the verifyItem list is smaller
132+ for (int i = 0; i < 12; ++i)
133+ model->removeLast();
134+
135+ model->insertString(0, "200");
136+
137+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 3);
138+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 2);
139+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 4);
140+ QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 3);
141+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, true);
142+ verifyItem(vj->m_columnVisibleItems[1][0], 1, 160, 0, true);
143+ verifyItem(vj->m_columnVisibleItems[2][0], 2, 320, 0, true);
144+ verifyItem(vj->m_columnVisibleItems[2][1], 3, 320, 60, true);
145+ verifyItem(vj->m_columnVisibleItems[1][1], 4, 160, 110, true);
146+ verifyItem(vj->m_columnVisibleItems[1][2], 5, 160, 130, true);
147+ verifyItem(vj->m_columnVisibleItems[1][3], 6, 160, 180, true);
148+ verifyItem(vj->m_columnVisibleItems[2][2], 7, 320, 195, true);
149+ verifyItem(vj->m_columnVisibleItems[0][1], 8, 0, 210, true);
150+ QCOMPARE(vj->implicitHeight(), 395.);
151+ }
152+
153+ void testChangeItem()
154+ {
155+ // Remove a few items from the end so the verifyItem list is smaller
156+ for (int i = 0; i < 11; ++i)
157+ model->removeLast();
158+
159+ model->changeString(0, "200");
160+
161+ QTRY_COMPARE(vj->m_columnVisibleItems.count(), 3);
162+ QTRY_COMPARE(vj->m_columnVisibleItems[0].count(), 2);
163+ QTRY_COMPARE(vj->m_columnVisibleItems[1].count(), 5);
164+ QTRY_COMPARE(vj->m_columnVisibleItems[2].count(), 2);
165+ verifyItem(vj->m_columnVisibleItems[0][0], 0, 0, 0, true);
166+ verifyItem(vj->m_columnVisibleItems[1][0], 1, 160, 0, true);
167+ verifyItem(vj->m_columnVisibleItems[2][0], 2, 320, 0, true);
168+ verifyItem(vj->m_columnVisibleItems[1][1], 3, 160, 60, true);
169+ verifyItem(vj->m_columnVisibleItems[1][2], 4, 160, 80, true);
170+ verifyItem(vj->m_columnVisibleItems[1][3], 5, 160, 130, true);
171+ verifyItem(vj->m_columnVisibleItems[2][1], 6, 320, 135, true);
172+ verifyItem(vj->m_columnVisibleItems[0][1], 7, 0, 210, true);
173+ verifyItem(vj->m_columnVisibleItems[1][4], 8, 160, 210, true);
174+ QCOMPARE(vj->implicitHeight(), 370.);
175+ }
176+
177 private:
178 QQuickView *view;
179 VerticalJournal *vj;

Subscribers

People subscribed via source and target branches