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
1=== modified file 'qml/Dash/Card.qml'
2--- qml/Dash/Card.qml 2014-01-30 21:25:10 +0000
3+++ qml/Dash/Card.qml 2014-02-10 10:01:00 +0000
4@@ -23,8 +23,13 @@
5 property var components
6 property var cardData
7
8+ property alias fontScale: header.fontScale
9+ readonly property alias headerHeight: header.height
10+
11+ property bool showHeader: true
12+
13 implicitWidth: childrenRect.width
14- implicitHeight: childrenRect.height
15+ implicitHeight: summary.y + summary.height
16
17 UbuntuShape {
18 id: artShape
19@@ -58,21 +63,77 @@
20 }
21 }
22
23+ ShaderEffect {
24+ id: overlay
25+ anchors {
26+ left: artShape.left
27+ right: artShape.right
28+ bottom: artShape.bottom
29+ }
30+
31+ height: header.height
32+ opacity: header.opacity * 0.6
33+ visible: template && template["overlay"] && artShape.visible && artShape.image.status === Image.Ready || false
34+
35+ property var source: ShaderEffectSource {
36+ id: shaderSource
37+ sourceItem: artShape
38+ onVisibleChanged: if (visible) scheduleUpdate()
39+ live: false
40+ sourceRect: Qt.rect(0, artShape.height - overlay.height, artShape.width, overlay.height)
41+ }
42+
43+ vertexShader: "
44+ uniform highp mat4 qt_Matrix;
45+ attribute highp vec4 qt_Vertex;
46+ attribute highp vec2 qt_MultiTexCoord0;
47+ varying highp vec2 coord;
48+ void main() {
49+ coord = qt_MultiTexCoord0;
50+ gl_Position = qt_Matrix * qt_Vertex;
51+ }"
52+
53+ fragmentShader: "
54+ varying highp vec2 coord;
55+ uniform sampler2D source;
56+ uniform lowp float qt_Opacity;
57+ void main() {
58+ lowp vec4 tex = texture2D(source, coord);
59+ gl_FragColor = vec4(0, 0, 0, tex.a) * qt_Opacity;
60+ }"
61+ }
62+
63 CardHeader {
64 id: header
65 objectName: "cardHeader"
66 anchors {
67- top: template && template["card-layout"] === "horizontal" ? artShape.top : artShape.bottom
68- left: template && template["card-layout"] === "horizontal" ? artShape.right : parent.left
69+ top: {
70+ if (template) {
71+ if (template["overlay"]) return overlay.top;
72+ if (template["card-layout"] === "horizontal") return artShape.top;
73+ }
74+ return artShape.bottom;
75+ }
76+ left: {
77+ if (template) {
78+ if (!template["overlay"] && template["card-layout"] === "horizontal") return artShape.right;
79+ }
80+ return parent.left;
81+ }
82 right: parent.right
83 }
84
85 mascot: cardData && cardData["mascot"] || ""
86 title: cardData && cardData["title"] || ""
87 subtitle: cardData && cardData["subtitle"] || ""
88+
89+ opacity: showHeader ? 1 : 0
90+
91+ Behavior on opacity { NumberAnimation { duration: UbuntuAnimation.SnapDuration } }
92 }
93
94 Label {
95+ id: summary
96 objectName: "summaryLabel"
97 anchors { top: header.visible ? header.bottom : artShape.bottom; left: parent.left; right: parent.right }
98 wrapMode: Text.Wrap
99
100=== modified file 'qml/Dash/CardHeader.qml'
101--- qml/Dash/CardHeader.qml 2014-02-04 14:08:13 +0000
102+++ qml/Dash/CardHeader.qml 2014-02-10 10:01:00 +0000
103@@ -26,6 +26,9 @@
104 property alias oldPrice: oldPriceLabel.text
105 property alias altPrice: altPriceLabel.text
106
107+ // FIXME: Saviq, used to scale fonts down in Carousel
108+ property real fontScale: 1.0
109+
110 visible: mascotImage.status === Image.Ready || title || price
111 height: row.height > 0 ? row.height + row.margins * 2 : 0
112
113@@ -41,7 +44,7 @@
114 leftMargin: spacing
115 rightMargin: spacing
116 }
117- spacing: mascotShape.visible ? margins : 0
118+ spacing: mascotShape.visible || (template && template["overlay"]) ? margins : 0
119
120 UbuntuShape {
121 id: mascotShape
122@@ -77,6 +80,8 @@
123 wrapMode: Text.Wrap
124 maximumLineCount: 2
125 fontSize: "small"
126+ font.pixelSize: Math.round(FontUtils.sizeToPixels(fontSize) * fontScale)
127+ color: template["overlay"] === true ? "white" : Theme.palette.selected.backgroundText
128 }
129
130 Label {
131@@ -87,6 +92,8 @@
132 font.weight: Font.Light
133 visible: titleLabel.text && text
134 fontSize: "x-small"
135+ font.pixelSize: Math.round(FontUtils.sizeToPixels(fontSize) * fontScale)
136+ color: template["overlay"] === true ? "white" : Theme.palette.selected.backgroundText
137 }
138
139 Row {
140
141=== modified file 'qml/Dash/CardTool.qml'
142--- qml/Dash/CardTool.qml 2014-02-03 09:41:39 +0000
143+++ qml/Dash/CardTool.qml 2014-02-10 10:01:00 +0000
144@@ -100,10 +100,14 @@
145 }
146 }
147
148- readonly property QtObject priv: card
149+ /*!
150+ type:real \brief Height of the card's header.
151+ */
152+ readonly property alias headerHeight: card.headerHeight
153
154 Card {
155 id: card
156+ objectName: "card"
157 template: cardTool.template
158 components: cardTool.components
159
160
161=== modified file 'tests/qmltests/Dash/tst_Card.qml'
162--- tests/qmltests/Dash/tst_Card.qml 2014-01-31 13:37:31 +0000
163+++ tests/qmltests/Dash/tst_Card.qml 2014-02-10 10:01:00 +0000
164@@ -74,12 +74,18 @@
165 "name": "Header title only",
166 "layout": { "components": { "title": "title" } }
167 },
168+ {
169+ "name": "Art, header, summary - overlaid",
170+ "layout": { "template": { "overlay": true },
171+ "components": JSON.parse(Helpers.fullMapping) }
172+ },
173 ]
174
175 CardTool {
176 id: cardTool
177 template: card.template
178 components: card.components
179+ viewWidth: units.gu(48)
180 }
181
182 Card {
183@@ -224,6 +230,7 @@
184 }
185
186 function test_card_size(data) {
187+ waitForRendering(card);
188 selector.selectedIndex = data.index;
189
190 if (data.hasOwnProperty("size")) {
191@@ -314,6 +321,8 @@
192 left: function() { return art.x }, index: 0 },
193 { tag: "Horizontal", top: function() { return art.y },
194 left: function() { return art.x + art.width }, index: 5 },
195+ { tag: "Overlay", top: function() { return art.y + art.height - header.height },
196+ left: function() { return art.x }, index: 9 },
197 ]
198 }
199
200
201=== modified file 'tests/qmltests/Dash/tst_CardTool.qml'
202--- tests/qmltests/Dash/tst_CardTool.qml 2014-01-31 16:23:32 +0000
203+++ tests/qmltests/Dash/tst_CardTool.qml 2014-02-10 10:01:00 +0000
204@@ -209,7 +209,7 @@
205 id: dataArea
206 anchors { left: parent.left; right: parent.right }
207 height: units.gu(25)
208- text: JSON.stringify(cardTool.priv.cardData, undefined, 2)
209+ text: JSON.stringify(testCase.internalCard.cardData, undefined, 2)
210 }
211
212 Label {
213@@ -239,8 +239,14 @@
214 id: testCase
215 name: "Card"
216
217+ property Card internalCard: findChild(cardTool, "card")
218+
219 when: windowShown
220
221+ function init() {
222+ verify(typeof testCase.internalCard === "object", "Couldn't find internal card object.");
223+ }
224+
225 function cleanup() {
226 selector.selectedIndex = -1;
227 layoutSelector.selectedIndex = -1;

Subscribers

People subscribed via source and target branches