Merge lp:~macslow/unity8/fix-1200569 into lp:unity8

Proposed by Mirco Müller
Status: Merged
Approved by: Michał Sawicz
Approved revision: 430
Merged at revision: 549
Proposed branch: lp:~macslow/unity8/fix-1200569
Merge into: lp:unity8
Diff against target: 211 lines (+99/-7)
3 files modified
Notifications/Notification.qml (+3/-7)
Notifications/ShapedIcon.qml (+40/-0)
tests/qmltests/Notifications/tst_Notifications.qml (+56/-0)
To merge this branch: bzr merge lp:~macslow/unity8/fix-1200569
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Michael Zanetti (community) Approve
Michał Sawicz Needs Fixing
Review via email: mp+190365@code.launchpad.net

Commit message

Description of the change

Added support for no-shape icon hint to fix bug #1200569.

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Michał Sawicz (saviq) wrote :

ShapedIcon.qml UNKNOWN *No copyright*

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

This should be doable with just one Image {}, one that you assign to UbuntuShape's image: prop if shaped, undefined otherwise.

Also, the added test is manual, please add checks for UbuntuShape's visibility and image !== undefined.

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

I don't think the ShapedIcon needs the wrapping Item. Can we just directly use the UbuntuShape there?

review: Needs Information
Revision history for this message
Mirco Müller (macslow) wrote :

Sure it can, but I thought you'd like that better separated out like this... anyway I can change it.

Revision history for this message
Mirco Müller (macslow) wrote :

No wait... that wouldn't work, if the UbuntuShape is the wrapping element. It would always be masked/shaped in that case, which is what the non-shape-icon hint is meant to control.

Revision history for this message
Michael Zanetti (mzanetti) wrote :

> No wait... that wouldn't work, if the UbuntuShape is the wrapping element. It
> would always be masked/shaped in that case, which is what the non-shape-icon
> hint is meant to control.

Ah, got it...

lgtm then.

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

FAILED: Continuous integration, rev:430
http://jenkins.qa.ubuntu.com/job/unity8-ci/1682/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/920
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/904
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/330
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/205
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/206
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/206/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/205
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/828
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/920
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/920/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/904
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/904/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3503
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/1593

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

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/743/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3351
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/934
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/918/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/338
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/129
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/129
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/129/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/129
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/838
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/934
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/934/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/918
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/918/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3514/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/1606

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

FAILED: Continuous integration, rev:430
http://jenkins.qa.ubuntu.com/job/unity8-ci/1688/
Executed test runs:
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/943
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/927
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/345
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/211
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/212
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/212/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/211
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/847
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/943
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/943/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/927
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/927/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3522
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/1621

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

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

FAILED: Autolanding.
More details in the following jenkins job:
http://jenkins.qa.ubuntu.com/job/unity8-autolanding/757/
Executed test runs:
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/generic-cleanup-mbs/3373
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/985
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/969
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/370
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-autolanding/143
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/143
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-autolanding/143/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-autolanding/143
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/883
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/985
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/985/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/969
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/969/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/3561
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/1664

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'Notifications/Notification.qml'
2--- Notifications/Notification.qml 2013-10-16 11:36:17 +0000
3+++ Notifications/Notification.qml 2013-11-20 15:44:50 +0000
4@@ -23,7 +23,7 @@
5 UbuntuShape {
6 id: notification
7
8- property alias iconSource: avatarIcon.source
9+ property alias iconSource: icon.fileSource
10 property alias secondaryIconSource: secondaryIcon.source
11 property alias summary: summaryLabel.text
12 property alias body: bodyLabel.text
13@@ -102,18 +102,14 @@
14 right: parent.right
15 }
16
17- UbuntuShape {
18+ ShapedIcon {
19 id: icon
20
21 objectName: "icon"
22 width: units.gu(6)
23 height: units.gu(6)
24+ shaped: notification.hints["x-canonical-non-shaped-icon"] == "true" ? false : true
25 visible: iconSource !== undefined && iconSource != ""
26- image: Image {
27- id: avatarIcon
28-
29- fillMode: Image.PreserveAspectCrop
30- }
31 }
32
33 Image {
34
35=== added file 'Notifications/ShapedIcon.qml'
36--- Notifications/ShapedIcon.qml 1970-01-01 00:00:00 +0000
37+++ Notifications/ShapedIcon.qml 2013-11-20 15:44:50 +0000
38@@ -0,0 +1,40 @@
39+/*
40+ * Copyright (C) 2013 Canonical, Ltd.
41+ *
42+ * This program is free software; you can redistribute it and/or modify
43+ * it under the terms of the GNU General Public License as published by
44+ * the Free Software Foundation; version 3.
45+ *
46+ * This program is distributed in the hope that it will be useful,
47+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
48+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
49+ * GNU General Public License for more details.
50+ *
51+ * You should have received a copy of the GNU General Public License
52+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
53+ */
54+
55+import QtQuick 2.0
56+import Ubuntu.Components 0.1
57+
58+Item {
59+ property string fileSource
60+ property bool shaped
61+
62+ UbuntuShape {
63+ objectName: "shapedIcon"
64+ anchors.fill: parent
65+ visible: shaped
66+ image: realImage
67+ }
68+
69+ Image {
70+ id: realImage
71+
72+ objectName: "nonShapedIcon"
73+ anchors.fill: parent
74+ visible: !shaped
75+ source: fileSource
76+ fillMode: Image.PreserveAspectCrop
77+ }
78+}
79
80=== modified file 'tests/qmltests/Notifications/tst_Notifications.qml'
81--- tests/qmltests/Notifications/tst_Notifications.qml 2013-10-08 10:02:05 +0000
82+++ tests/qmltests/Notifications/tst_Notifications.qml 2013-11-20 15:44:50 +0000
83@@ -76,6 +76,20 @@
84 mockModel.append(n)
85 }
86
87+ function addEphemeralNonShapedIconNotification() {
88+ var n = {
89+ type: Notification.Ephemeral,
90+ hints: {"x-canonical-non-shaped-icon": "true"},
91+ summary: "Contacts",
92+ body: "Synchronised contacts-database with cloud-storage.",
93+ icon: "../graphics/applicationIcons/contacts-app.png",
94+ secondaryIcon: "",
95+ actions: []
96+ }
97+
98+ mockModel.append(n)
99+ }
100+
101 function addEphemeralIconSummaryNotification() {
102 var n = {
103 type: Notification.Ephemeral,
104@@ -158,6 +172,12 @@
105
106 Button {
107 width: parent.width
108+ text: "add an non-shaped-icon-summary-body"
109+ onClicked: addEphemeralNonShapedIconNotification()
110+ }
111+
112+ Button {
113+ width: parent.width
114 text: "add an icon-summary"
115 onClicked: addEphemeralIconSummaryNotification()
116 }
117@@ -206,6 +226,8 @@
118 bodyVisible: true,
119 interactiveAreaEnabled: false,
120 iconVisible: true,
121+ shapedIcon: true,
122+ nonShapedIcon: false,
123 secondaryIconVisible: true,
124 buttonRowVisible: true,
125 buttonTinted: true
126@@ -223,6 +245,8 @@
127 bodyVisible: false,
128 interactiveAreaEnabled: false,
129 iconVisible: false,
130+ shapedIcon: false,
131+ nonShapedIcon: false,
132 secondaryIconVisible: true,
133 buttonRowVisible: false,
134 buttonTinted: false
135@@ -240,6 +264,8 @@
136 bodyVisible: false,
137 interactiveAreaEnabled: false,
138 iconVisible: false,
139+ shapedIcon: false,
140+ nonShapedIcon: false,
141 secondaryIconVisible: true,
142 buttonRowVisible: false,
143 buttonTinted: false
144@@ -257,6 +283,8 @@
145 bodyVisible: true,
146 interactiveAreaEnabled: true,
147 iconVisible: true,
148+ shapedIcon: true,
149+ nonShapedIcon: false,
150 secondaryIconVisible: false,
151 buttonRowVisible: false,
152 buttonTinted: false
153@@ -275,6 +303,8 @@
154 bodyVisible: true,
155 interactiveAreaEnabled: false,
156 iconVisible: true,
157+ shapedIcon: true,
158+ nonShapedIcon: false,
159 secondaryIconVisible: false,
160 buttonRowVisible: true,
161 buttonTinted: false
162@@ -292,9 +322,31 @@
163 bodyVisible: true,
164 interactiveAreaEnabled: false,
165 iconVisible: true,
166+ shapedIcon: true,
167+ nonShapedIcon: false,
168 secondaryIconVisible: true,
169 buttonRowVisible: false,
170 buttonTinted: false
171+ },
172+ {
173+ tag: "Ephemeral notification with non-shaped icon",
174+ type: Notification.Ephemeral,
175+ hints: {"x-canonical-private-button-tint": "false",
176+ "x-canonical-non-shaped-icon": "true"},
177+ summary: "Contacts",
178+ body: "Synchronised contacts-database with cloud-storage.",
179+ icon: "../graphics/applicationIcons/contacts-app.png",
180+ secondaryIcon: "",
181+ actions: [],
182+ summaryVisible: true,
183+ bodyVisible: true,
184+ interactiveAreaEnabled: false,
185+ iconVisible: true,
186+ shapedIcon: false,
187+ nonShapedIcon: true,
188+ secondaryIconVisible: false,
189+ buttonRowVisible: false,
190+ buttonTinted: false
191 }
192 ]
193 }
194@@ -330,6 +382,8 @@
195 verify(notification !== undefined, "notification wasn't found");
196
197 var icon = findChild(notification, "icon")
198+ var shapedIcon = findChild(notification, "shapedIcon")
199+ var nonShapedIcon = findChild(notification, "nonShapedIcon")
200 var interactiveArea = findChild(notification, "interactiveArea")
201 var secondaryIcon = findChild(notification, "secondaryIcon")
202 var summaryLabel = findChild(notification, "summaryLabel")
203@@ -338,6 +392,8 @@
204 waitForRendering(buttonRow)
205
206 compare(icon.visible, data.iconVisible, "avatar-icon visibility is incorrect")
207+ compare(shapedIcon.visible, data.shapedIcon, "shaped-icon visibility is incorrect")
208+ compare(nonShapedIcon.visible, data.nonShapedIcon, "non-shaped-icon visibility is incorrect")
209 compare(interactiveArea.enabled, data.interactiveAreaEnabled, "check for interactive area")
210
211 if(data.interactiveAreaEnabled) {

Subscribers

People subscribed via source and target branches