Merge lp:~unity-team/unity8/new-scopes-vj-integration into lp:~unity-team/unity8/new-scopes

Proposed by Michał Karnicki
Status: Superseded
Proposed branch: lp:~unity-team/unity8/new-scopes-vj-integration
Merge into: lp:~unity-team/unity8/new-scopes
Diff against target: 405 lines (+339/-3)
7 files modified
plugins/DashViews/abstractjournal.cpp (+1/-1)
plugins/DashViews/verticaljournal.cpp (+1/-1)
qml/Components/ResponsiveVerticalJournal.qml (+68/-0)
qml/Dash/CardVerticalJournal.qml (+60/-0)
qml/Dash/GenericScopeView.qml (+2/-1)
tests/qmltests/CMakeLists.txt (+1/-0)
tests/qmltests/Components/tst_ResponsiveVerticalJournal.qml (+206/-0)
To merge this branch: bzr merge lp:~unity-team/unity8/new-scopes-vj-integration
Reviewer Review Type Date Requested Status
Albert Astals Cid (community) Needs Information
Michał Sawicz Needs Fixing
Review via email: mp+201932@code.launchpad.net

This proposal has been superseded by a proposal from 2014-04-08.

Commit message

Add VerticalJournal integration.

Description of the change

Add VerticalJournal integration.

To post a comment you must log in.
Revision history for this message
Michał Karnicki (karni) wrote :

PS the abstractjournal.cpp and verticaljournal.cpp changes come from Albert, and have been merged in a different branch already. I added them to avoid crashers.

Revision history for this message
Michał Sawicz (saviq) wrote :

Number of columns should be dynamic based on delegate width.

review: Needs Fixing
583. By Michał Karnicki

Don't try to fix the header title positioning with hardcorded value.

584. By Michał Karnicki

Fix copyright headers.

585. By Michał Karnicki

Make column count dynamic.

586. By Michał Karnicki

Update CardVerticalJournal defaults.

Revision history for this message
Michał Karnicki (karni) wrote :
Revision history for this message
Michał Karnicki (karni) wrote :

unity-scope-tool shows nothing, when JSON updated to: 'category-layout': 'vertical-journal' :(

587. By Michał Karnicki

Bind ResponsiveVJ model and height.

588. By Michał Karnicki

Fetch template and components from DashRenderer. Set RVJ columnWidth cased on card size from template.

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

Why left, right and top margin but no bottom margin?

review: Needs Information
589. By Michał Karnicki

Add bottom margin.

Revision history for this message
Michał Karnicki (karni) wrote :

I followed ResponsiveGridView implementation as reference. Added bottom margin.

590. By Michał Karnicki

Expand CardVerticalJournal to fill its parent.

Revision history for this message
Michał Karnicki (karni) wrote :

Getting seg fault'ed when I add the filtering capability. Will have to leave this to Saviq.

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

I'm sure we can debug that crash without Saviq, which stacktrace are you getting? How easy is it to reproduce?

Revision history for this message
Michał Karnicki (karni) wrote :

lp:~unity-team/unity8/new-scopes-vj-integration-filter
$ ./build -c
$ ./builddir/tools/unity-scope-tool
Override category -> set category-layout to "vertical-journal", apply.
Notice seg fault.

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

Does not segfault here :S

Are you doing any other change? Or do you have uncommited changes?

Revision history for this message
Michał Sawicz (saviq) wrote :

Can you rebase (except for CardVerticalJournal) onto lp:unity8 at some point please?

Unmerged revisions

590. By Michał Karnicki

Expand CardVerticalJournal to fill its parent.

589. By Michał Karnicki

Add bottom margin.

588. By Michał Karnicki

Fetch template and components from DashRenderer. Set RVJ columnWidth cased on card size from template.

587. By Michał Karnicki

Bind ResponsiveVJ model and height.

586. By Michał Karnicki

Update CardVerticalJournal defaults.

585. By Michał Karnicki

Make column count dynamic.

584. By Michał Karnicki

Fix copyright headers.

583. By Michał Karnicki

Don't try to fix the header title positioning with hardcorded value.

582. By Michał Karnicki

Fix column and row spacing tests.

581. By Michał Karnicki

Add ResponsiveVerticalJournal tests. column and row spacing still not passing.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'plugins/DashViews/abstractjournal.cpp'
2--- plugins/DashViews/abstractjournal.cpp 2014-01-07 14:51:15 +0000
3+++ plugins/DashViews/abstractjournal.cpp 2014-01-20 23:24:53 +0000
4@@ -194,7 +194,7 @@
5
6 void AbstractJournal::refill()
7 {
8- if (!isComponentComplete()) {
9+ if (!isComponentComplete() || height() < 0) {
10 return;
11 }
12
13
14=== modified file 'plugins/DashViews/verticaljournal.cpp'
15--- plugins/DashViews/verticaljournal.cpp 2014-01-03 09:18:24 +0000
16+++ plugins/DashViews/verticaljournal.cpp 2014-01-20 23:24:53 +0000
17@@ -79,7 +79,7 @@
18
19 void VerticalJournal::findTopModelIndexToAdd(int *modelIndex, qreal *yPos)
20 {
21- *modelIndex = INT_MAX;
22+ *modelIndex = 0;
23 *yPos = std::numeric_limits<qreal>::lowest();
24 int columnToAddTo = -1;
25
26
27=== added file 'qml/Components/ResponsiveVerticalJournal.qml'
28--- qml/Components/ResponsiveVerticalJournal.qml 1970-01-01 00:00:00 +0000
29+++ qml/Components/ResponsiveVerticalJournal.qml 2014-01-20 23:24:53 +0000
30@@ -0,0 +1,68 @@
31+/*
32+ * Copyright (C) 2013-2014 Canonical, Ltd.
33+ *
34+ * This program is free software; you can redistribute it and/or modify
35+ * it under the terms of the GNU General Public License as published by
36+ * the Free Software Foundation; version 3.
37+ *
38+ * This program is distributed in the hope that it will be useful,
39+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
40+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
41+ * GNU General Public License for more details.
42+ *
43+ * You should have received a copy of the GNU General Public License
44+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
45+ */
46+
47+import QtQuick 2.0
48+import Ubuntu.Components 0.1
49+import "../Components"
50+import DashViews 0.1
51+
52+// A VerticalJournal, which allows setting max number of columns. Based on
53+// defined column width, delegates are spread in the horizontal space.
54+Item {
55+ property int minimumColumnSpacing: units.gu(0.5)
56+ property int maximumNumberOfColumns: 2
57+
58+ property alias columnWidth: verticalJournalView.columnWidth
59+ property alias rowSpacing: verticalJournalView.rowSpacing
60+ property alias model: verticalJournalView.model
61+ property alias delegate: verticalJournalView.delegate
62+ property alias delegateCreationBegin: verticalJournalView.delegateCreationBegin
63+ property alias delegateCreationEnd: verticalJournalView.delegateCreationEnd
64+
65+ //readonly property alias currentItem // TODO bug #1269024
66+
67+ VerticalJournal {
68+ id: verticalJournalView
69+ objectName: "responsiveVerticalJournalView"
70+ anchors {
71+ fill: parent
72+ leftMargin: columnSpacing/2
73+ rightMargin: columnSpacing/2
74+ topMargin: rowSpacing/2
75+ bottomMargin: rowSpacing/2
76+ }
77+ clip: parent.height != implicitHeight
78+
79+ function px2gu(pixels) {
80+ return Math.floor(pixels / units.gu(1))
81+ }
82+
83+ function columnsForSpacing(spacing) {
84+ // parent.width = columns * columnWidth +
85+ // (columns-1) * spacing + spacing(margins)
86+ return Math.max(1, Math.floor(parent.width / (columnWidth + spacing)))
87+ }
88+
89+ function spacingForColumns(columns) {
90+ var spacingGU = px2gu((parent.width - columns * columnWidth) / columns)
91+ return units.gu(spacingGU)
92+ }
93+
94+ property int expectedColumns: Math.min(
95+ columnsForSpacing(minimumColumnSpacing), maximumNumberOfColumns)
96+ columnSpacing: spacingForColumns(expectedColumns)
97+ }
98+}
99
100=== added file 'qml/Dash/CardVerticalJournal.qml'
101--- qml/Dash/CardVerticalJournal.qml 1970-01-01 00:00:00 +0000
102+++ qml/Dash/CardVerticalJournal.qml 2014-01-20 23:24:53 +0000
103@@ -0,0 +1,60 @@
104+/*
105+ * Copyright (C) 2013 Canonical, Ltd.
106+ *
107+ * This program is free software; you can redistribute it and/or modify
108+ * it under the terms of the GNU General Public License as published by
109+ * the Free Software Foundation; version 3.
110+ *
111+ * This program is distributed in the hope that it will be useful,
112+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
113+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
114+ * GNU General Public License for more details.
115+ *
116+ * You should have received a copy of the GNU General Public License
117+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
118+ */
119+
120+import QtQuick 2.0
121+import Ubuntu.Components 0.1
122+import "../Components"
123+
124+DashRenderer {
125+ id: genericVerticalJournal
126+
127+ property alias minimumColumnSpacing: cardVerticalJournal.minimumColumnSpacing
128+ property alias maximumNumberOfColumns: cardVerticalJournal.maximumNumberOfColumns
129+ property alias columnWidth: cardVerticalJournal.columnWidth
130+ property alias rowSpacing: cardVerticalJournal.rowSpacing
131+
132+ anchors.fill: parent
133+
134+ ResponsiveVerticalJournal {
135+ id: cardVerticalJournal
136+ anchors.fill: parent
137+ maximumNumberOfColumns: 2
138+ minimumColumnSpacing: units.gu(1)
139+ rowSpacing: units.gu(1)
140+ model: genericVerticalJournal.model
141+
142+ columnWidth: {
143+ if (genericVerticalJournal.template !== undefined) {
144+ switch (genericVerticalJournal.template['card-size']) {
145+ case "small": return units.gu(12);
146+ case "large": return units.gu(38);
147+ }
148+ }
149+ return units.gu(18.5);
150+ }
151+
152+ delegate: Card {
153+ id: card
154+ objectName: "delegate" + index
155+ cardData: model
156+ template: genericVerticalJournal.template
157+ components: genericVerticalJournal.components
158+
159+ //onClicked: cardVerticalJournal.clicked(index, tile.y)
160+ //onPressAndHold: cardVerticalJournal.pressAndHold(index, tile.y)
161+ }
162+ }
163+}
164
165=== modified file 'qml/Dash/GenericScopeView.qml'
166--- qml/Dash/GenericScopeView.qml 2014-01-09 09:05:57 +0000
167+++ qml/Dash/GenericScopeView.qml 2014-01-20 23:24:53 +0000
168@@ -1,5 +1,5 @@
169 /*
170- * Copyright (C) 2013 Canonical, Ltd.
171+ * Copyright (C) 2013-2014 Canonical, Ltd.
172 *
173 * This program is free software; you can redistribute it and/or modify
174 * it under the terms of the GNU General Public License as published by
175@@ -326,6 +326,7 @@
176 }
177 switch (layout) {
178 case "carousel": return "CardCarousel.qml";
179+ case "vertical-journal": return "CardVerticalJournal.qml";
180 case "grid":
181 default: return "CardFilterGrid.qml";
182 }
183
184=== modified file 'tests/qmltests/CMakeLists.txt'
185--- tests/qmltests/CMakeLists.txt 2014-01-16 14:12:43 +0000
186+++ tests/qmltests/CMakeLists.txt 2014-01-20 23:24:53 +0000
187@@ -31,6 +31,7 @@
188 add_qml_test(Components LazyImage)
189 add_qml_test(Components ResponsiveFlowView)
190 add_qml_test(Components ResponsiveGridView)
191+add_qml_test(Components ResponsiveVerticalJournal IMPORT_PATHS ${qmltest_DEFAULT_IMPORT_PATHS} ${CMAKE_BINARY_DIR}/plugins)
192 add_qml_test(Components Revealer)
193 add_qml_test(Components Showable)
194 add_qml_test(Components Stage IMPORT_PATHS ${qmltest_DEFAULT_IMPORT_PATHS}
195
196=== added file 'tests/qmltests/Components/tst_ResponsiveVerticalJournal.qml'
197--- tests/qmltests/Components/tst_ResponsiveVerticalJournal.qml 1970-01-01 00:00:00 +0000
198+++ tests/qmltests/Components/tst_ResponsiveVerticalJournal.qml 2014-01-20 23:24:53 +0000
199@@ -0,0 +1,206 @@
200+/*
201+ * Copyright 2013-2014 Canonical Ltd.
202+ *
203+ * This program is free software; you can redistribute it and/or modify
204+ * it under the terms of the GNU General Public License as published by
205+ * the Free Software Foundation; version 3.
206+ *
207+ * This program is distributed in the hope that it will be useful,
208+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
209+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
210+ * GNU General Public License for more details.
211+ *
212+ * You should have received a copy of the GNU General Public License
213+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
214+ */
215+
216+import QtQuick 2.0
217+import QtTest 1.0
218+import ".."
219+import "../../../qml/Components"
220+import Ubuntu.Components.ListItems 0.1 as ListItem
221+import Ubuntu.Components 0.1
222+import Utils 0.1
223+import Unity.Test 0.1 as UT
224+
225+Item {
226+ width: journalRect.width + controls.width
227+ height: units.gu(80)
228+
229+ Column {
230+ id: controls
231+ width: units.gu(40)
232+ height: parent.height
233+ anchors.top: parent.top
234+ anchors.right: parent.right
235+ ListItem.ValueSelector {
236+ id: cardSizeSelector
237+ text: "card-size"
238+ // small, medium, large card sizes
239+ values: [units.gu(12), units.gu(18.5), units.gu(38)]
240+ selectedIndex: 0
241+ }
242+ ListItem.ValueSelector {
243+ id: minColumnSpacingSelector
244+ text: "minColumnSpacing"
245+ values: [0, units.gu(2), units.gu(8), units.gu(25)]
246+ selectedIndex: 0
247+ }
248+ ListItem.ValueSelector {
249+ id: maxColumnsSelector
250+ text: "maxColumns"
251+ values: [1, 2, 3, 8, 15, fakeModel.count]
252+ selectedIndex: 1
253+ }
254+ ListItem.ValueSelector {
255+ id: rowSpacingSelector
256+ text: "rowSpacing"
257+ values: [units.gu(1), units.gu(2), units.gu(4), units.gu(8)]
258+ selectedIndex: 1
259+ }
260+ }
261+
262+ ListModel {
263+ id: fakeModel
264+ ListElement { name: "A" }
265+ ListElement { name: "B" }
266+ ListElement { name: "C" }
267+ ListElement { name: "D" }
268+ ListElement { name: "E" }
269+ ListElement { name: "F" }
270+ ListElement { name: "G" }
271+ ListElement { name: "H" }
272+ ListElement { name: "I" }
273+ ListElement { name: "J" }
274+ ListElement { name: "K" }
275+ ListElement { name: "L" }
276+ ListElement { name: "M" }
277+ ListElement { name: "N" }
278+ ListElement { name: "O" }
279+ ListElement { name: "P" }
280+ ListElement { name: "Q" }
281+ ListElement { name: "R" }
282+ ListElement { name: "S" }
283+ ListElement { name: "T" }
284+ ListElement { name: "U" }
285+ }
286+
287+ SortFilterProxyModel {
288+ id: wrappedFakeModel
289+ model: fakeModel
290+ }
291+
292+ Rectangle {
293+ id: journalRect
294+ width: units.gu(80)
295+ height: parent.height
296+ color: "grey"
297+ anchors.top: parent.top
298+ anchors.left: parent.left
299+
300+ ResponsiveVerticalJournal {
301+ id: journal
302+ anchors.fill: parent
303+ model: wrappedFakeModel
304+ minimumColumnSpacing: minColumnSpacingSelector.
305+ values[minColumnSpacingSelector.selectedIndex]
306+ maximumNumberOfColumns:
307+ maxColumnsSelector.values[maxColumnsSelector.selectedIndex]
308+ rowSpacing:
309+ rowSpacingSelector.values[rowSpacingSelector.selectedIndex]
310+ columnWidth: // XXX karni: How do I get that from the delegate?
311+ cardSizeSelector.values[cardSizeSelector.selectedIndex]
312+
313+ delegate: Rectangle {
314+ id: delegateItem
315+ // So that it can be identified by test code
316+ property bool isJournalDelegate: true
317+ objectName: "delegate" + index
318+ color: "grey"
319+ border.color: "red"
320+ border.width: 1
321+
322+ // width derived from Card's template['card-size']
323+ width: cardSizeSelector.values[cardSizeSelector.selectedIndex]
324+ height: Math.max(units.gu(8), Math.floor(Math.random() * 300))
325+
326+ Rectangle {
327+ color: "green"
328+ anchors.centerIn: parent
329+ width: units.gu(6)
330+ height: units.gu(6)
331+ Text {
332+ anchors.centerIn: parent
333+ text: name
334+ }
335+ }
336+
337+ Text { x:0; y:0; text:"(" + parent.x + ", " + parent.y + ")"}
338+ }
339+ }
340+ }
341+
342+ UT.UnityTestCase {
343+ name: "ResponsiveVerticalJournal"
344+ when: windowShown
345+
346+ function test_minimumColumnSpacing_data() {
347+ var data = new Array()
348+ data.push({minColumnSpacingIndex: 0, expectedColumns: 2})
349+ data.push({minColumnSpacingIndex: 1, expectedColumns: 2})
350+ data.push({minColumnSpacingIndex: 2, expectedColumns: 1})
351+ return data
352+ }
353+
354+ // Test how minimumColumnSpacing affects column count.
355+ function test_minimumColumnSpacing(data) {
356+ cardSizeSelector.selectedIndex = 2 // large card
357+ maxColumnsSelector.selectedIndex = 1 // two columns
358+
359+ minColumnSpacingSelector.selectedIndex = data.minColumnSpacingIndex
360+
361+ tryCompareFunction(countJournalDelegatesOnFirstRow, data.expectedColumns)
362+ }
363+
364+ function test_maximumNumberOfColumns_data() {
365+ var data = new Array()
366+ // Change maxColumns
367+ data.push({maxColumnsIndex: 0, cardSizeIndex: 0, expectedColumns: 1})
368+ data.push({maxColumnsIndex: 1, cardSizeIndex: 0, expectedColumns: 2})
369+ data.push({maxColumnsIndex: 2, cardSizeIndex: 0, expectedColumns: 3})
370+ data.push({maxColumnsIndex: 3, cardSizeIndex: 0, expectedColumns: 6})
371+ data.push({maxColumnsIndex: 4, cardSizeIndex: 0, expectedColumns: 6})
372+ // Change card size
373+ data.push({maxColumnsIndex: 3, cardSizeIndex: 1, expectedColumns: 4})
374+ data.push({maxColumnsIndex: 3, cardSizeIndex: 2, expectedColumns: 2})
375+ return data
376+ }
377+
378+ // Test how maximumNumberOfColumns and columnWidth affect column count.
379+ function test_maximumNumberOfColumns(data) {
380+ minColumnSpacingSelector.selectedIndex = 0 // no spacing
381+
382+ cardSizeSelector.selectedIndex = data.cardSizeIndex // columnWidth
383+ maxColumnsSelector.selectedIndex = data.maxColumnsIndex
384+
385+ tryCompareFunction(countJournalDelegatesOnFirstRow, data.expectedColumns)
386+ }
387+
388+ function countJournalDelegatesOnFirstRow() {
389+ return __countJournalDelegatesOnFirstRow(journal.visibleChildren, 0)
390+ }
391+
392+ function __countJournalDelegatesOnFirstRow(objList, total) {
393+ for (var i = 0; i < objList.length; ++i) {
394+ var child = objList[i];
395+ if (child.isJournalDelegate !== undefined && child.y === 0) {
396+ ++total;
397+ } else {
398+ total = __countJournalDelegatesOnFirstRow(
399+ child.visibleChildren, total)
400+ }
401+ }
402+ return total
403+ }
404+ }
405+}

Subscribers

People subscribed via source and target branches

to all changes: