Unity 8

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

  1. readonly_properties_to_cardtool
  2. Merge into trunk
Proposed by Albert Astals Cid
Status: Merged
Approved by: Andrea Cimitan
Approved revision: 2599
Merged at revision: 2676
Proposed branch: lp:~aacid/unity8/readonly_properties_to_cardtool
Merge into: lp:unity8
Diff against target: 251 lines (+65/-62)
3 files modified
qml/Dash/CardGrid.qml (+3/-4)
qml/Dash/CardTool.qml (+60/-16)
qml/Dash/GenericScopeView.qml (+2/-42)
To merge this branch: bzr merge lp:~aacid/unity8/readonly_properties_to_cardtool
Reviewer Review Type Date Requested Status
Andrea Cimitan (community) Approve
Unity8 CI Bot continuous-integration Approve
Review via email: mp+303431@code.launchpad.net

Commit message

Make more CardTool properties readonly

This removes "hacks" from GenericScopeView and moves them to either CardTool or CardGrid in a less "this is a hack" way

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.
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

FAILED: Continuous integration, rev:2598
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1992/
Executed test runs:
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build/2611/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2639
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/2518
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/2518
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=yakkety/2518
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2512
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2512/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2512
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2512/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2512
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2512/artifact/output/*zip*/output.zip
    FAILURE: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2512/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2512
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2512/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2512
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2512/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2512
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2512/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2512
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2512/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2512
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2512/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)
lp:~aacid/unity8/readonly_properties_to_cardtool updated
2599. By Albert Astals Cid

Merge

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

PASSED: Continuous integration, rev:2599
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/2257/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build/2971
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/1637
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/1637
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=yakkety,testname=qmluitests.sh/1637
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/2999
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2857
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/2857/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2857
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/2857/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2857
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=yakkety/2857/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2857
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/2857/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2857
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/2857/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2857
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=yakkety/2857/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2857
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/2857/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2857
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/2857/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2857
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=yakkety/2857/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)
Revision history for this message
Andrea Cimitan (cimi) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?
yes in a silo
 * Did CI run pass? If not, please explain why.
y

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'qml/Dash/CardGrid.qml'
--- qml/Dash/CardGrid.qml 2016-05-09 08:34:38 +0000
+++ qml/Dash/CardGrid.qml 2016-09-27 13:36:54 +0000
@@ -20,13 +20,12 @@
20DashRenderer {20DashRenderer {
21 id: root21 id: root
2222
23 readonly property real defaultMinimumHorizontalSpacing: units.gu(1)
24 readonly property int collapsedRows: {23 readonly property int collapsedRows: {
25 if (!cardTool || !cardTool.template || typeof cardTool.template["collapsed-rows"] != "number") return 2;24 if (!cardTool || !cardTool.template || typeof cardTool.template["collapsed-rows"] != "number") return 2;
26 return cardTool.template["collapsed-rows"];25 return cardTool.template["collapsed-rows"];
27 }26 }
28 property string backgroundShapeStyle: "inset";27 // ↓ This is the ubuntu store icon
29 property alias minimumHorizontalSpacing: grid.minimumHorizontalSpacing28 readonly property string backgroundShapeStyle: cardTool.isAppLikeScope && !cardTool.isAppLikeScopeAppCategory ? "shadow" : "inset"
3029
31 expandedHeight: grid.totalContentHeight30 expandedHeight: grid.totalContentHeight
32 collapsedHeight: Math.min(grid.contentHeightForRows(collapsedRows, grid.cellHeight), expandedHeight)31 collapsedHeight: Math.min(grid.contentHeightForRows(collapsedRows, grid.cellHeight), expandedHeight)
@@ -46,7 +45,7 @@
46 ResponsiveGridView {45 ResponsiveGridView {
47 id: grid46 id: grid
48 anchors.fill: parent47 anchors.fill: parent
49 minimumHorizontalSpacing: root.defaultMinimumHorizontalSpacing48 minimumHorizontalSpacing: (cardTool.isAppLikeScopeAppCategory && cardTool.isWideView) ? units.gu(5) : units.gu(1)
50 delegateWidth: cardTool.cardWidth49 delegateWidth: cardTool.cardWidth
51 delegateHeight: cardTool.cardHeight50 delegateHeight: cardTool.cardHeight
52 verticalSpacing: units.gu(1)51 verticalSpacing: units.gu(1)
5352
=== modified file 'qml/Dash/CardTool.qml'
--- qml/Dash/CardTool.qml 2016-08-23 12:48:23 +0000
+++ qml/Dash/CardTool.qml 2016-09-27 13:36:54 +0000
@@ -42,11 +42,6 @@
42 property real viewWidth42 property real viewWidth
4343
44 /*!44 /*!
45 \brief Scaling factor of selected Carousel item.
46 */
47 readonly property real carouselSelectedItemScaleFactor: 1.38 // XXX assuming 1.38 carousel scaling factor for cards
48
49 /*!
50 \brief Template supplied for the category.45 \brief Template supplied for the category.
51 */46 */
52 property var template47 property var template
@@ -57,9 +52,24 @@
57 property var components52 property var components
5853
59 /*!54 /*!
55 \brief The Scope this cardTool is representing
56 */
57 property string scopeId
58
59 /*!
60 \brief The Scope category this cardTool is representing
61 */
62 property string categoryId
63
64 /*!
65 \brief Scaling factor of selected Carousel item.
66 */
67 readonly property real carouselSelectedItemScaleFactor: 1.38 // XXX assuming 1.38 carousel scaling factor for cards
68
69 /*!
60 \brief The category layout for this card tool.70 \brief The category layout for this card tool.
61 */71 */
62 property string categoryLayout: {72 readonly property string categoryLayout: {
63 var layout = template["category-layout"];73 var layout = template["category-layout"];
6474
65 // carousel fallback mode to grid75 // carousel fallback mode to grid
@@ -67,9 +77,17 @@
67 return layout;77 return layout;
68 }78 }
6979
80 readonly property bool isAppLikeScope: scopeId === "clickscope" || scopeId === "libertine-scope.ubuntu_libertine-scope"
81 readonly property bool isAppLikeScopeAppCategory: ((scopeId === "clickscope" && (categoryId === "predefined" || categoryId === "local"))
82 || (scopeId === "libertine-scope.ubuntu_libertine-scope" && categoryId !== "hint"))
7083
71 // Not readonly because gets overwritten from GenericScopeView in some cases84 readonly property string artShapeStyle: {
72 property string artShapeStyle: categoryLayout === "carousel" ? "shadow" : "inset"85 if (isAppLikeScope) {
86 return isAppLikeScopeAppCategory ? "icon" : "flat";
87 } else {
88 return categoryLayout === "carousel" ? "shadow" : "inset"
89 }
90 }
7391
74 // FIXME ? This seems like it should not be needed, but on Qt 5.4 + phone92 // FIXME ? This seems like it should not be needed, but on Qt 5.4 + phone
75 // we are doing unneeded calls to getCardComponent with artShapeStyle and categoryLayout being empty93 // we are doing unneeded calls to getCardComponent with artShapeStyle and categoryLayout being empty
@@ -90,12 +108,31 @@
90 height: 0108 height: 0
91 clip: true109 clip: true
92110
111 // We have 3 view "widths"
112 // narrow <= 45gu
113 // normal
114 // wide >= 70gu
115 readonly property bool isWideView: viewWidth >= units.gu(70)
116 readonly property bool isNarrowView: viewWidth <= units.gu(45)
117
93 /*!118 /*!
94 type:real \brief Width to be enforced on the card in this configuration.119 type:real \brief Width to be enforced on the card in this configuration.
95120
96 If -1, should use implicit width of the actual card.121 If -1, should use implicit width of the actual card.
97 */122 */
98 property real cardWidth: {123 readonly property real cardWidth: {
124 if (isAppLikeScopeAppCategory) {
125 if (!isNarrowView) {
126 if (isWideView) {
127 return units.gu(11);
128 } else {
129 return units.gu(10);
130 }
131 } else {
132 return units.gu(12);
133 }
134 }
135
99 switch (categoryLayout) {136 switch (categoryLayout) {
100 case "grid":137 case "grid":
101 case "vertical-journal":138 case "vertical-journal":
@@ -103,16 +140,16 @@
103 if (template["card-layout"] === "horizontal") size = "large";140 if (template["card-layout"] === "horizontal") size = "large";
104 switch (size) {141 switch (size) {
105 case "small": {142 case "small": {
106 if (viewWidth <= units.gu(45)) return units.gu(12);143 if (isNarrowView) return units.gu(12);
107 else return units.gu(14);144 else return units.gu(14);
108 }145 }
109 case "large": {146 case "large": {
110 if (viewWidth >= units.gu(70)) return units.gu(42);147 if (isWideView) return units.gu(42);
111 else return viewWidth - units.gu(2);148 else return viewWidth - units.gu(2);
112 }149 }
113 }150 }
114 if (viewWidth <= units.gu(45)) return units.gu(18);151 if (isNarrowView) return units.gu(18);
115 else if (viewWidth >= units.gu(70)) return units.gu(20);152 else if (isWideView) return units.gu(20);
116 else return units.gu(23);153 else return units.gu(23);
117 case "carousel":154 case "carousel":
118 case "horizontal-list":155 case "horizontal-list":
@@ -152,12 +189,19 @@
152 type:real \brief Height of the card's header.189 type:real \brief Height of the card's header.
153 */190 */
154 readonly property int headerHeight: cardLoader.item ? cardLoader.item.headerHeight : 0191 readonly property int headerHeight: cardLoader.item ? cardLoader.item.headerHeight : 0
155 property size artShapeSize: cardLoader.item ? cardLoader.item.artShapeSize : Qt.size(0, 0)192
193 readonly property size artShapeSize: {
194 if (isAppLikeScopeAppCategory) {
195 return Qt.size(units.gu(8), units.gu(7.5));
196 } else {
197 return cardLoader.item ? cardLoader.item.artShapeSize : Qt.size(0, 0)
198 }
199 }
156200
157 QtObject {201 QtObject {
158 id: carouselTool202 id: carouselTool
159203
160 property real minimumTileWidth: {204 readonly property real minimumTileWidth: {
161 if (cardTool.viewWidth === undefined) return undefined;205 if (cardTool.viewWidth === undefined) return undefined;
162 if (cardTool.viewWidth <= units.gu(40)) return units.gu(18);206 if (cardTool.viewWidth <= units.gu(40)) return units.gu(18);
163 if (cardTool.viewWidth >= units.gu(128)) return units.gu(26);207 if (cardTool.viewWidth >= units.gu(128)) return units.gu(26);
@@ -166,7 +210,7 @@
166210
167 readonly property real pathItemCount: 4.8457 /// (848 / 175) reference values211 readonly property real pathItemCount: 4.8457 /// (848 / 175) reference values
168212
169 property real realPathItemCount: {213 readonly property real realPathItemCount: {
170 var scaledMinimumTileWidth = minimumTileWidth / cardTool.carouselSelectedItemScaleFactor;214 var scaledMinimumTileWidth = minimumTileWidth / cardTool.carouselSelectedItemScaleFactor;
171 var tileWidth = Math.max(cardTool.viewWidth / pathItemCount, scaledMinimumTileWidth);215 var tileWidth = Math.max(cardTool.viewWidth / pathItemCount, scaledMinimumTileWidth);
172 return Math.min(cardTool.viewWidth / tileWidth, pathItemCount);216 return Math.min(cardTool.viewWidth / tileWidth, pathItemCount);
173217
=== modified file 'qml/Dash/GenericScopeView.qml'
--- qml/Dash/GenericScopeView.qml 2016-09-22 07:37:21 +0000
+++ qml/Dash/GenericScopeView.qml 2016-09-27 13:36:54 +0000
@@ -267,6 +267,8 @@
267 template: model.renderer267 template: model.renderer
268 components: model.components268 components: model.components
269 viewWidth: parent.width269 viewWidth: parent.width
270 scopeId: scope ? scope.id : ""
271 categoryId: baseItem.category
270 }272 }
271273
272 onExpandableChanged: {274 onExpandableChanged: {
@@ -342,17 +344,6 @@
342 baseItem.expand(shouldExpand, false /*animate*/);344 baseItem.expand(shouldExpand, false /*animate*/);
343 }345 }
344 updateRanges();346 updateRanges();
345 clickScopeSizingHacks();
346 if (scope && (scope.id === "clickscope" || scope.id === "libertine-scope.ubuntu_libertine-scope")) {
347 if (isLibertineContainerCategory() || categoryId === "predefined" || categoryId === "local") {
348 cardTool.artShapeSize = Qt.binding(function() { return Qt.size(units.gu(8), units.gu(7.5)) });
349 cardTool.artShapeStyle = "icon";
350 } else {
351 // Should be ubuntu store icon
352 cardTool.artShapeStyle = "flat";
353 item.backgroundShapeStyle = "shadow";
354 }
355 }
356 item.cardTool = cardTool;347 item.cardTool = cardTool;
357 }348 }
358349
@@ -361,36 +352,6 @@
361 scopeView.enableHeightBehaviorOnNextCreation = item.enableHeightBehaviorOnNextCreation;352 scopeView.enableHeightBehaviorOnNextCreation = item.enableHeightBehaviorOnNextCreation;
362 }353 }
363 }354 }
364 // FIXME: directly connecting to onUnitsChanged cause a compile error:
365 // Cannot assign to non-existent property "onUnitsChanged"
366 // Until the units object is reworked to properly do all we need, let's go through a intermediate property
367 property int pxpgu: units.gu(1);
368 onPxpguChanged: clickScopeSizingHacks();
369
370 // Returns true if the current category pertains to a Libertine container
371 function isLibertineContainerCategory() {
372 return scope && scope.id === "libertine-scope.ubuntu_libertine-scope" && categoryId !== "hint";
373 }
374
375 function clickScopeSizingHacks() {
376 if (scope &&
377 ((scope.id === "clickscope" && (categoryId === "predefined" || categoryId === "local")) ||
378 isLibertineContainerCategory())) {
379 // Yeah, hackish :/
380 if (scopeView.width > units.gu(45)) {
381 if (scopeView.width >= units.gu(70)) {
382 cardTool.cardWidth = units.gu(11);
383 item.minimumHorizontalSpacing = units.gu(5);
384 return;
385 } else {
386 cardTool.cardWidth = units.gu(10);
387 }
388 } else {
389 cardTool.cardWidth = units.gu(12);
390 }
391 item.minimumHorizontalSpacing = item.defaultMinimumHorizontalSpacing;
392 }
393 }
394355
395 Connections {356 Connections {
396 target: rendererLoader.item357 target: rendererLoader.item
@@ -425,7 +386,6 @@
425 target: scopeView386 target: scopeView
426 onIsCurrentChanged: rendererLoader.updateRanges();387 onIsCurrentChanged: rendererLoader.updateRanges();
427 onVisibleToParentChanged: rendererLoader.updateRanges();388 onVisibleToParentChanged: rendererLoader.updateRanges();
428 onWidthChanged: rendererLoader.clickScopeSizingHacks();
429 }389 }
430 Connections {390 Connections {
431 target: holdingList391 target: holdingList

Subscribers

People subscribed via source and target branches