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

Proposed by Albert Astals Cid
Status: Rejected
Rejected by: Albert Astals Cid
Proposed branch: lp:~unity-team/unity8/new-scopes-vj-integration
Merge into: lp:~unity-team/unity8/new-scopes
Diff against target: 381 lines (+337/-1)
5 files modified
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 Fixing
Michał Sawicz Pending
Review via email: mp+214786@code.launchpad.net

This proposal supersedes a proposal from 2014-01-16.

Description of the change

Add VerticalJournal integration.

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

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 : Posted in a previous version of this proposal

Number of columns should be dynamic based on delegate width.

review: Needs Fixing
Revision history for this message
Michał Karnicki (karni) wrote : Posted in a previous version of this proposal
Revision history for this message
Michał Karnicki (karni) wrote : Posted in a previous version of this proposal

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

Revision history for this message
Albert Astals Cid (aacid) wrote : Posted in a previous version of this proposal

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

review: Needs Information
Revision history for this message
Michał Karnicki (karni) wrote : Posted in a previous version of this proposal

I followed ResponsiveGridView implementation as reference. Added bottom margin.

Revision history for this message
Michał Karnicki (karni) wrote : Posted in a previous version of this proposal

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 : Posted in a previous version of this proposal

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 : Posted in a previous version of this proposal

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 : Posted in a previous version of this proposal

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 : Posted in a previous version of this proposal

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

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

Needs resubmission against lp:unity8

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

We don't do new-scopes anymore

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
=== modified file 'plugins/DashViews/abstractdashview.cpp'
=== modified file 'plugins/DashViews/verticaljournal.cpp'
=== added file 'qml/Components/ResponsiveVerticalJournal.qml'
--- qml/Components/ResponsiveVerticalJournal.qml 1970-01-01 00:00:00 +0000
+++ qml/Components/ResponsiveVerticalJournal.qml 2014-04-08 15:12:51 +0000
@@ -0,0 +1,68 @@
1/*
2 * Copyright (C) 2013-2014 Canonical, Ltd.
3 *
4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License as published by
6 * the Free Software Foundation; version 3.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17import QtQuick 2.0
18import Ubuntu.Components 0.1
19import "../Components"
20import DashViews 0.1
21
22// A VerticalJournal, which allows setting max number of columns. Based on
23// defined column width, delegates are spread in the horizontal space.
24Item {
25 property int minimumColumnSpacing: units.gu(0.5)
26 property int maximumNumberOfColumns: 2
27
28 property alias columnWidth: verticalJournalView.columnWidth
29 property alias rowSpacing: verticalJournalView.rowSpacing
30 property alias model: verticalJournalView.model
31 property alias delegate: verticalJournalView.delegate
32 property alias delegateCreationBegin: verticalJournalView.delegateCreationBegin
33 property alias delegateCreationEnd: verticalJournalView.delegateCreationEnd
34
35 //readonly property alias currentItem // TODO bug #1269024
36
37 VerticalJournal {
38 id: verticalJournalView
39 objectName: "responsiveVerticalJournalView"
40 anchors {
41 fill: parent
42 leftMargin: columnSpacing/2
43 rightMargin: columnSpacing/2
44 topMargin: rowSpacing/2
45 bottomMargin: rowSpacing/2
46 }
47 clip: parent.height != implicitHeight
48
49 function px2gu(pixels) {
50 return Math.floor(pixels / units.gu(1))
51 }
52
53 function columnsForSpacing(spacing) {
54 // parent.width = columns * columnWidth +
55 // (columns-1) * spacing + spacing(margins)
56 return Math.max(1, Math.floor(parent.width / (columnWidth + spacing)))
57 }
58
59 function spacingForColumns(columns) {
60 var spacingGU = px2gu((parent.width - columns * columnWidth) / columns)
61 return units.gu(spacingGU)
62 }
63
64 property int expectedColumns: Math.min(
65 columnsForSpacing(minimumColumnSpacing), maximumNumberOfColumns)
66 columnSpacing: spacingForColumns(expectedColumns)
67 }
68}
069
=== added file 'qml/Dash/CardVerticalJournal.qml'
--- qml/Dash/CardVerticalJournal.qml 1970-01-01 00:00:00 +0000
+++ qml/Dash/CardVerticalJournal.qml 2014-04-08 15:12:51 +0000
@@ -0,0 +1,60 @@
1/*
2 * Copyright (C) 2013 Canonical, Ltd.
3 *
4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License as published by
6 * the Free Software Foundation; version 3.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17import QtQuick 2.0
18import Ubuntu.Components 0.1
19import "../Components"
20
21DashRenderer {
22 id: genericVerticalJournal
23
24 property alias minimumColumnSpacing: cardVerticalJournal.minimumColumnSpacing
25 property alias maximumNumberOfColumns: cardVerticalJournal.maximumNumberOfColumns
26 property alias columnWidth: cardVerticalJournal.columnWidth
27 property alias rowSpacing: cardVerticalJournal.rowSpacing
28
29 anchors.fill: parent
30
31 ResponsiveVerticalJournal {
32 id: cardVerticalJournal
33 anchors.fill: parent
34 maximumNumberOfColumns: 2
35 minimumColumnSpacing: units.gu(1)
36 rowSpacing: units.gu(1)
37 model: genericVerticalJournal.model
38
39 columnWidth: {
40 if (genericVerticalJournal.template !== undefined) {
41 switch (genericVerticalJournal.template['card-size']) {
42 case "small": return units.gu(12);
43 case "large": return units.gu(38);
44 }
45 }
46 return units.gu(18.5);
47 }
48
49 delegate: Card {
50 id: card
51 objectName: "delegate" + index
52 cardData: model
53 template: genericVerticalJournal.template
54 components: genericVerticalJournal.components
55
56 //onClicked: cardVerticalJournal.clicked(index, tile.y)
57 //onPressAndHold: cardVerticalJournal.pressAndHold(index, tile.y)
58 }
59 }
60}
061
=== modified file 'qml/Dash/GenericScopeView.qml'
--- qml/Dash/GenericScopeView.qml 2014-03-06 11:06:07 +0000
+++ qml/Dash/GenericScopeView.qml 2014-04-08 15:12:51 +0000
@@ -1,5 +1,5 @@
1/*1/*
2 * Copyright (C) 2013 Canonical, Ltd.2 * Copyright (C) 2013-2014 Canonical, Ltd.
3 *3 *
4 * This program is free software; you can redistribute it and/or modify4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License as published by5 * it under the terms of the GNU General Public License as published by
@@ -339,6 +339,7 @@
339 }339 }
340 switch (layout) {340 switch (layout) {
341 case "carousel": return "CardCarousel.qml";341 case "carousel": return "CardCarousel.qml";
342 case "vertical-journal": return "CardVerticalJournal.qml";
342 case "grid":343 case "grid":
343 default: return "CardFilterGrid.qml";344 default: return "CardFilterGrid.qml";
344 }345 }
345346
=== modified file 'tests/qmltests/CMakeLists.txt'
--- tests/qmltests/CMakeLists.txt 2014-03-03 12:24:20 +0000
+++ tests/qmltests/CMakeLists.txt 2014-04-08 15:12:51 +0000
@@ -32,6 +32,7 @@
32add_qml_test(Components Rating)32add_qml_test(Components Rating)
33add_qml_test(Components ResponsiveFlowView)33add_qml_test(Components ResponsiveFlowView)
34add_qml_test(Components ResponsiveGridView)34add_qml_test(Components ResponsiveGridView)
35add_qml_test(Components ResponsiveVerticalJournal IMPORT_PATHS ${qmltest_DEFAULT_IMPORT_PATHS} ${CMAKE_BINARY_DIR}/plugins)
35add_qml_test(Components Revealer)36add_qml_test(Components Revealer)
36add_qml_test(Components SeeMore)37add_qml_test(Components SeeMore)
37add_qml_test(Components Showable)38add_qml_test(Components Showable)
3839
=== added file 'tests/qmltests/Components/tst_ResponsiveVerticalJournal.qml'
--- tests/qmltests/Components/tst_ResponsiveVerticalJournal.qml 1970-01-01 00:00:00 +0000
+++ tests/qmltests/Components/tst_ResponsiveVerticalJournal.qml 2014-04-08 15:12:51 +0000
@@ -0,0 +1,206 @@
1/*
2 * Copyright 2013-2014 Canonical Ltd.
3 *
4 * This program is free software; you can redistribute it and/or modify
5 * it under the terms of the GNU General Public License as published by
6 * the Free Software Foundation; version 3.
7 *
8 * This program is distributed in the hope that it will be useful,
9 * but WITHOUT ANY WARRANTY; without even the implied warranty of
10 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11 * GNU General Public License for more details.
12 *
13 * You should have received a copy of the GNU General Public License
14 * along with this program. If not, see <http://www.gnu.org/licenses/>.
15 */
16
17import QtQuick 2.0
18import QtTest 1.0
19import ".."
20import "../../../qml/Components"
21import Ubuntu.Components.ListItems 0.1 as ListItem
22import Ubuntu.Components 0.1
23import Utils 0.1
24import Unity.Test 0.1 as UT
25
26Item {
27 width: journalRect.width + controls.width
28 height: units.gu(80)
29
30 Column {
31 id: controls
32 width: units.gu(40)
33 height: parent.height
34 anchors.top: parent.top
35 anchors.right: parent.right
36 ListItem.ValueSelector {
37 id: cardSizeSelector
38 text: "card-size"
39 // small, medium, large card sizes
40 values: [units.gu(12), units.gu(18.5), units.gu(38)]
41 selectedIndex: 0
42 }
43 ListItem.ValueSelector {
44 id: minColumnSpacingSelector
45 text: "minColumnSpacing"
46 values: [0, units.gu(2), units.gu(8), units.gu(25)]
47 selectedIndex: 0
48 }
49 ListItem.ValueSelector {
50 id: maxColumnsSelector
51 text: "maxColumns"
52 values: [1, 2, 3, 8, 15, fakeModel.count]
53 selectedIndex: 1
54 }
55 ListItem.ValueSelector {
56 id: rowSpacingSelector
57 text: "rowSpacing"
58 values: [units.gu(1), units.gu(2), units.gu(4), units.gu(8)]
59 selectedIndex: 1
60 }
61 }
62
63 ListModel {
64 id: fakeModel
65 ListElement { name: "A" }
66 ListElement { name: "B" }
67 ListElement { name: "C" }
68 ListElement { name: "D" }
69 ListElement { name: "E" }
70 ListElement { name: "F" }
71 ListElement { name: "G" }
72 ListElement { name: "H" }
73 ListElement { name: "I" }
74 ListElement { name: "J" }
75 ListElement { name: "K" }
76 ListElement { name: "L" }
77 ListElement { name: "M" }
78 ListElement { name: "N" }
79 ListElement { name: "O" }
80 ListElement { name: "P" }
81 ListElement { name: "Q" }
82 ListElement { name: "R" }
83 ListElement { name: "S" }
84 ListElement { name: "T" }
85 ListElement { name: "U" }
86 }
87
88 SortFilterProxyModel {
89 id: wrappedFakeModel
90 model: fakeModel
91 }
92
93 Rectangle {
94 id: journalRect
95 width: units.gu(80)
96 height: parent.height
97 color: "grey"
98 anchors.top: parent.top
99 anchors.left: parent.left
100
101 ResponsiveVerticalJournal {
102 id: journal
103 anchors.fill: parent
104 model: wrappedFakeModel
105 minimumColumnSpacing: minColumnSpacingSelector.
106 values[minColumnSpacingSelector.selectedIndex]
107 maximumNumberOfColumns:
108 maxColumnsSelector.values[maxColumnsSelector.selectedIndex]
109 rowSpacing:
110 rowSpacingSelector.values[rowSpacingSelector.selectedIndex]
111 columnWidth: // XXX karni: How do I get that from the delegate?
112 cardSizeSelector.values[cardSizeSelector.selectedIndex]
113
114 delegate: Rectangle {
115 id: delegateItem
116 // So that it can be identified by test code
117 property bool isJournalDelegate: true
118 objectName: "delegate" + index
119 color: "grey"
120 border.color: "red"
121 border.width: 1
122
123 // width derived from Card's template['card-size']
124 width: cardSizeSelector.values[cardSizeSelector.selectedIndex]
125 height: Math.max(units.gu(8), Math.floor(Math.random() * 300))
126
127 Rectangle {
128 color: "green"
129 anchors.centerIn: parent
130 width: units.gu(6)
131 height: units.gu(6)
132 Text {
133 anchors.centerIn: parent
134 text: name
135 }
136 }
137
138 Text { x:0; y:0; text:"(" + parent.x + ", " + parent.y + ")"}
139 }
140 }
141 }
142
143 UT.UnityTestCase {
144 name: "ResponsiveVerticalJournal"
145 when: windowShown
146
147 function test_minimumColumnSpacing_data() {
148 var data = new Array()
149 data.push({minColumnSpacingIndex: 0, expectedColumns: 2})
150 data.push({minColumnSpacingIndex: 1, expectedColumns: 2})
151 data.push({minColumnSpacingIndex: 2, expectedColumns: 1})
152 return data
153 }
154
155 // Test how minimumColumnSpacing affects column count.
156 function test_minimumColumnSpacing(data) {
157 cardSizeSelector.selectedIndex = 2 // large card
158 maxColumnsSelector.selectedIndex = 1 // two columns
159
160 minColumnSpacingSelector.selectedIndex = data.minColumnSpacingIndex
161
162 tryCompareFunction(countJournalDelegatesOnFirstRow, data.expectedColumns)
163 }
164
165 function test_maximumNumberOfColumns_data() {
166 var data = new Array()
167 // Change maxColumns
168 data.push({maxColumnsIndex: 0, cardSizeIndex: 0, expectedColumns: 1})
169 data.push({maxColumnsIndex: 1, cardSizeIndex: 0, expectedColumns: 2})
170 data.push({maxColumnsIndex: 2, cardSizeIndex: 0, expectedColumns: 3})
171 data.push({maxColumnsIndex: 3, cardSizeIndex: 0, expectedColumns: 6})
172 data.push({maxColumnsIndex: 4, cardSizeIndex: 0, expectedColumns: 6})
173 // Change card size
174 data.push({maxColumnsIndex: 3, cardSizeIndex: 1, expectedColumns: 4})
175 data.push({maxColumnsIndex: 3, cardSizeIndex: 2, expectedColumns: 2})
176 return data
177 }
178
179 // Test how maximumNumberOfColumns and columnWidth affect column count.
180 function test_maximumNumberOfColumns(data) {
181 minColumnSpacingSelector.selectedIndex = 0 // no spacing
182
183 cardSizeSelector.selectedIndex = data.cardSizeIndex // columnWidth
184 maxColumnsSelector.selectedIndex = data.maxColumnsIndex
185
186 tryCompareFunction(countJournalDelegatesOnFirstRow, data.expectedColumns)
187 }
188
189 function countJournalDelegatesOnFirstRow() {
190 return __countJournalDelegatesOnFirstRow(journal.visibleChildren, 0)
191 }
192
193 function __countJournalDelegatesOnFirstRow(objList, total) {
194 for (var i = 0; i < objList.length; ++i) {
195 var child = objList[i];
196 if (child.isJournalDelegate !== undefined && child.y === 0) {
197 ++total;
198 } else {
199 total = __countJournalDelegatesOnFirstRow(
200 child.visibleChildren, total)
201 }
202 }
203 return total
204 }
205 }
206}

Subscribers

People subscribed via source and target branches

to all changes: