Merge lp:~unity-team/unity8/unity8-card-overlay into lp:unity8

Proposed by Michał Karnicki
Status: Merged
Approved by: Albert Astals Cid
Approved revision: 698
Merged at revision: 703
Proposed branch: lp:~unity-team/unity8/unity8-card-overlay
Merge into: lp:unity8
Diff against target: 227 lines (+93/-6)
5 files modified
qml/Dash/Card.qml (+64/-3)
qml/Dash/CardHeader.qml (+8/-1)
qml/Dash/CardTool.qml (+5/-1)
tests/qmltests/Dash/tst_Card.qml (+9/-0)
tests/qmltests/Dash/tst_CardTool.qml (+7/-1)
To merge this branch: bzr merge lp:~unity-team/unity8/unity8-card-overlay
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Needs Fixing
Albert Astals Cid (community) Approve
Michał Sawicz Needs Fixing
Michał Karnicki (community) Approve
Review via email: mp+204790@code.launchpad.net

Commit message

* Add overlay to card.
* Fix implicit card height.

Description of the change

* Add overlay to card.
* Fix implicit card height.

Saviq's code, so I'll do the review.

Are there any related MPs required for this MP to build/function as expected? Please list.
 * 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?
 * Will be reviewed in a bigger context.

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

This code was written by Saviq, so I'm +1'ing it after reviewing it.

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

* it should only show up on the current item in Carousel, not all of them
* height should probably be dynamic, based on maximum header size
* we need some margins, too
* we need to scale down fonts if carousel + overlay, because items are scaled up (as opposed to down, as expected by dash visual designs)

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Karnicki (karni) wrote :

* height should probably be dynamic, based on maximum header size
  DONE.

Rest fixed in newscopes-card-overlay.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:690
http://jenkins.qa.ubuntu.com/job/unity8-ci/2261/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/2982
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2715
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1132
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/783
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/785
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/785/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/783
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2621
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2984
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/2984/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2716
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2716/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/5139
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3717

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2261/rebuild

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

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes, tryCard works fine

 * Did CI run pass? If not, please explain why.
There's an autopilot failure unrelated to this

review: Approve
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:692
http://jenkins.qa.ubuntu.com/job/unity8-ci/2271/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/3007
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/2739/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1142
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/793
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/795
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/795/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/793
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/2643
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/3009
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/3009/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2740
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/2740/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/5158/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/3744

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/unity8-ci/2271/rebuild

review: Needs Fixing (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'qml/Dash/Card.qml'
--- qml/Dash/Card.qml 2014-01-30 21:25:10 +0000
+++ qml/Dash/Card.qml 2014-02-10 10:01:00 +0000
@@ -23,8 +23,13 @@
23 property var components23 property var components
24 property var cardData24 property var cardData
2525
26 property alias fontScale: header.fontScale
27 readonly property alias headerHeight: header.height
28
29 property bool showHeader: true
30
26 implicitWidth: childrenRect.width31 implicitWidth: childrenRect.width
27 implicitHeight: childrenRect.height32 implicitHeight: summary.y + summary.height
2833
29 UbuntuShape {34 UbuntuShape {
30 id: artShape35 id: artShape
@@ -58,21 +63,77 @@
58 }63 }
59 }64 }
6065
66 ShaderEffect {
67 id: overlay
68 anchors {
69 left: artShape.left
70 right: artShape.right
71 bottom: artShape.bottom
72 }
73
74 height: header.height
75 opacity: header.opacity * 0.6
76 visible: template && template["overlay"] && artShape.visible && artShape.image.status === Image.Ready || false
77
78 property var source: ShaderEffectSource {
79 id: shaderSource
80 sourceItem: artShape
81 onVisibleChanged: if (visible) scheduleUpdate()
82 live: false
83 sourceRect: Qt.rect(0, artShape.height - overlay.height, artShape.width, overlay.height)
84 }
85
86 vertexShader: "
87 uniform highp mat4 qt_Matrix;
88 attribute highp vec4 qt_Vertex;
89 attribute highp vec2 qt_MultiTexCoord0;
90 varying highp vec2 coord;
91 void main() {
92 coord = qt_MultiTexCoord0;
93 gl_Position = qt_Matrix * qt_Vertex;
94 }"
95
96 fragmentShader: "
97 varying highp vec2 coord;
98 uniform sampler2D source;
99 uniform lowp float qt_Opacity;
100 void main() {
101 lowp vec4 tex = texture2D(source, coord);
102 gl_FragColor = vec4(0, 0, 0, tex.a) * qt_Opacity;
103 }"
104 }
105
61 CardHeader {106 CardHeader {
62 id: header107 id: header
63 objectName: "cardHeader"108 objectName: "cardHeader"
64 anchors {109 anchors {
65 top: template && template["card-layout"] === "horizontal" ? artShape.top : artShape.bottom110 top: {
66 left: template && template["card-layout"] === "horizontal" ? artShape.right : parent.left111 if (template) {
112 if (template["overlay"]) return overlay.top;
113 if (template["card-layout"] === "horizontal") return artShape.top;
114 }
115 return artShape.bottom;
116 }
117 left: {
118 if (template) {
119 if (!template["overlay"] && template["card-layout"] === "horizontal") return artShape.right;
120 }
121 return parent.left;
122 }
67 right: parent.right123 right: parent.right
68 }124 }
69125
70 mascot: cardData && cardData["mascot"] || ""126 mascot: cardData && cardData["mascot"] || ""
71 title: cardData && cardData["title"] || ""127 title: cardData && cardData["title"] || ""
72 subtitle: cardData && cardData["subtitle"] || ""128 subtitle: cardData && cardData["subtitle"] || ""
129
130 opacity: showHeader ? 1 : 0
131
132 Behavior on opacity { NumberAnimation { duration: UbuntuAnimation.SnapDuration } }
73 }133 }
74134
75 Label {135 Label {
136 id: summary
76 objectName: "summaryLabel"137 objectName: "summaryLabel"
77 anchors { top: header.visible ? header.bottom : artShape.bottom; left: parent.left; right: parent.right }138 anchors { top: header.visible ? header.bottom : artShape.bottom; left: parent.left; right: parent.right }
78 wrapMode: Text.Wrap139 wrapMode: Text.Wrap
79140
=== modified file 'qml/Dash/CardHeader.qml'
--- qml/Dash/CardHeader.qml 2014-02-04 14:08:13 +0000
+++ qml/Dash/CardHeader.qml 2014-02-10 10:01:00 +0000
@@ -26,6 +26,9 @@
26 property alias oldPrice: oldPriceLabel.text26 property alias oldPrice: oldPriceLabel.text
27 property alias altPrice: altPriceLabel.text27 property alias altPrice: altPriceLabel.text
2828
29 // FIXME: Saviq, used to scale fonts down in Carousel
30 property real fontScale: 1.0
31
29 visible: mascotImage.status === Image.Ready || title || price32 visible: mascotImage.status === Image.Ready || title || price
30 height: row.height > 0 ? row.height + row.margins * 2 : 033 height: row.height > 0 ? row.height + row.margins * 2 : 0
3134
@@ -41,7 +44,7 @@
41 leftMargin: spacing44 leftMargin: spacing
42 rightMargin: spacing45 rightMargin: spacing
43 }46 }
44 spacing: mascotShape.visible ? margins : 047 spacing: mascotShape.visible || (template && template["overlay"]) ? margins : 0
4548
46 UbuntuShape {49 UbuntuShape {
47 id: mascotShape50 id: mascotShape
@@ -77,6 +80,8 @@
77 wrapMode: Text.Wrap80 wrapMode: Text.Wrap
78 maximumLineCount: 281 maximumLineCount: 2
79 fontSize: "small"82 fontSize: "small"
83 font.pixelSize: Math.round(FontUtils.sizeToPixels(fontSize) * fontScale)
84 color: template["overlay"] === true ? "white" : Theme.palette.selected.backgroundText
80 }85 }
8186
82 Label {87 Label {
@@ -87,6 +92,8 @@
87 font.weight: Font.Light92 font.weight: Font.Light
88 visible: titleLabel.text && text93 visible: titleLabel.text && text
89 fontSize: "x-small"94 fontSize: "x-small"
95 font.pixelSize: Math.round(FontUtils.sizeToPixels(fontSize) * fontScale)
96 color: template["overlay"] === true ? "white" : Theme.palette.selected.backgroundText
90 }97 }
9198
92 Row {99 Row {
93100
=== modified file 'qml/Dash/CardTool.qml'
--- qml/Dash/CardTool.qml 2014-02-03 09:41:39 +0000
+++ qml/Dash/CardTool.qml 2014-02-10 10:01:00 +0000
@@ -100,10 +100,14 @@
100 }100 }
101 }101 }
102102
103 readonly property QtObject priv: card103 /*!
104 type:real \brief Height of the card's header.
105 */
106 readonly property alias headerHeight: card.headerHeight
104107
105 Card {108 Card {
106 id: card109 id: card
110 objectName: "card"
107 template: cardTool.template111 template: cardTool.template
108 components: cardTool.components112 components: cardTool.components
109113
110114
=== modified file 'tests/qmltests/Dash/tst_Card.qml'
--- tests/qmltests/Dash/tst_Card.qml 2014-01-31 13:37:31 +0000
+++ tests/qmltests/Dash/tst_Card.qml 2014-02-10 10:01:00 +0000
@@ -74,12 +74,18 @@
74 "name": "Header title only",74 "name": "Header title only",
75 "layout": { "components": { "title": "title" } }75 "layout": { "components": { "title": "title" } }
76 },76 },
77 {
78 "name": "Art, header, summary - overlaid",
79 "layout": { "template": { "overlay": true },
80 "components": JSON.parse(Helpers.fullMapping) }
81 },
77 ]82 ]
7883
79 CardTool {84 CardTool {
80 id: cardTool85 id: cardTool
81 template: card.template86 template: card.template
82 components: card.components87 components: card.components
88 viewWidth: units.gu(48)
83 }89 }
8490
85 Card {91 Card {
@@ -224,6 +230,7 @@
224 }230 }
225231
226 function test_card_size(data) {232 function test_card_size(data) {
233 waitForRendering(card);
227 selector.selectedIndex = data.index;234 selector.selectedIndex = data.index;
228235
229 if (data.hasOwnProperty("size")) {236 if (data.hasOwnProperty("size")) {
@@ -314,6 +321,8 @@
314 left: function() { return art.x }, index: 0 },321 left: function() { return art.x }, index: 0 },
315 { tag: "Horizontal", top: function() { return art.y },322 { tag: "Horizontal", top: function() { return art.y },
316 left: function() { return art.x + art.width }, index: 5 },323 left: function() { return art.x + art.width }, index: 5 },
324 { tag: "Overlay", top: function() { return art.y + art.height - header.height },
325 left: function() { return art.x }, index: 9 },
317 ]326 ]
318 }327 }
319328
320329
=== modified file 'tests/qmltests/Dash/tst_CardTool.qml'
--- tests/qmltests/Dash/tst_CardTool.qml 2014-01-31 16:23:32 +0000
+++ tests/qmltests/Dash/tst_CardTool.qml 2014-02-10 10:01:00 +0000
@@ -209,7 +209,7 @@
209 id: dataArea209 id: dataArea
210 anchors { left: parent.left; right: parent.right }210 anchors { left: parent.left; right: parent.right }
211 height: units.gu(25)211 height: units.gu(25)
212 text: JSON.stringify(cardTool.priv.cardData, undefined, 2)212 text: JSON.stringify(testCase.internalCard.cardData, undefined, 2)
213 }213 }
214214
215 Label {215 Label {
@@ -239,8 +239,14 @@
239 id: testCase239 id: testCase
240 name: "Card"240 name: "Card"
241241
242 property Card internalCard: findChild(cardTool, "card")
243
242 when: windowShown244 when: windowShown
243245
246 function init() {
247 verify(typeof testCase.internalCard === "object", "Couldn't find internal card object.");
248 }
249
244 function cleanup() {250 function cleanup() {
245 selector.selectedIndex = -1;251 selector.selectedIndex = -1;
246 layoutSelector.selectedIndex = -1;252 layoutSelector.selectedIndex = -1;

Subscribers

People subscribed via source and target branches