Merge lp:~saviq/ubuntu-ui-toolkit/add-icon-source-property into lp:ubuntu-ui-toolkit/staging

Proposed by Michał Sawicz
Status: Merged
Approved by: Michał Sawicz
Approved revision: 1133
Merged at revision: 1173
Proposed branch: lp:~saviq/ubuntu-ui-toolkit/add-icon-source-property
Merge into: lp:ubuntu-ui-toolkit/staging
Diff against target: 426 lines (+122/-116)
6 files modified
components.api (+3/-0)
modules/Ubuntu/Components/Icon.qdoc (+87/-0)
modules/Ubuntu/Components/Icon10.qml (+8/-53)
modules/Ubuntu/Components/Icon11.qml (+21/-0)
modules/Ubuntu/Components/qmldir (+3/-2)
tests/unit_x11/tst_components/tst_icon.qml (+0/-61)
To merge this branch: bzr merge lp:~saviq/ubuntu-ui-toolkit/add-icon-source-property
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve
Tim Peeters Approve
Zsombor Egri Needs Fixing
Review via email: mp+224122@code.launchpad.net

Commit message

Add "source" property to the Icon component.

I didn't alias the source of Image to support resetting Icon.source, and also I didn't think exposing the image://theme/... uri was needed.

To post a comment you must log in.
Revision history for this message
Tim Peeters (tpeeters) wrote :

24 + * Copyright (C) 2013 Canonical, Ltd.

2014

Revision history for this message
Tim Peeters (tpeeters) wrote :

move all the docs to Icon.qdoc, because qdoc seems to use the filename for the component name in documentation. So now "make docs" creates separate documentation for Icon10 and Icon11.

review: Needs Fixing
Revision history for this message
Tim Peeters (tpeeters) wrote :

+ source: ready && width > 0 && height > 0 ? (icon.source || (icon.name ? "image://theme/%1".arg(icon.name) : "")) : ""

if icon.source is set to "", will the image source be "" then or an image from the theme?

Please document the behavior when both source and name are set (do we need a unit test to check the behavior also?)

Revision history for this message
Tim Peeters (tpeeters) wrote :

In Action.qml, we define this:

    property url iconSource: iconName ? "image://theme/" + iconName : ""

we could follow the same idea in the icon:

property url source: name ? "image://theme/" + name : ""

and simply set image.source to icon.source. What do you think?

Revision history for this message
Tim Peeters (tpeeters) wrote :

> + source: ready && width > 0 && height > 0 ? (icon.source || (icon.name
> ? "image://theme/%1".arg(icon.name) : "")) : ""
>
> if icon.source is set to "", will the image source be "" then or an image from
> the theme?

Of course I can find out what is the answer of my question, but what I tried to say is that the code is not immediately unambiguous when you first read it. But my suggestion to use something like this:
 property url iconSource: iconName ? "image://theme/" + iconName : ""
would avoid this problem anyway.

Revision history for this message
Zsombor Egri (zsombi) wrote :

Also, don't forget to add unit tests as well.

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

PASSED: Continuous integration, rev:1117
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/436/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/1106
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/988
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-ci/268
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/268
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-ci/268/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-ci/268
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/1472
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1927
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/1927/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/8731
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/843
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1131
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1131/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/436/rebuild

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

All comments attended to. Please let me know if you'd like to test something more, for example.

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

CI fails because ./test/qmlapicheck.sh fails with Error: Missing \qmlproperty for color

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

Not sure what to do... \qmlproperty is there in .qdoc for the color prop...

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
Tim Peeters (tpeeters) wrote :

********* Start testing of components *********
Config: Using QtTest library 5.3.0, Qt 5.3.0
PASS : components::Icon::initTestCase()
FAIL! : components::Icon::test_name() Source of the image should be image://theme/{name}.
   Actual ():
   Expected (): image://theme/foo
   Loc: [/tmp/buildd/ubuntu-ui-toolkit-0.1.47+14.10.20140618bzr1125pkg0utopic1194+autopilot0/tests/unit_x11/tst_components/tst_icon.qml(42)]
FAIL! : components::Icon::test_source() Source of the image should equal icon.source.
   Actual ():
   Expected (): file://foo/bar
   Loc: [/tmp/buildd/ubuntu-ui-toolkit-0.1.47+14.10.20140618bzr1125pkg0utopic1194+autopilot0/tests/unit_x11/tst_components/tst_icon.qml(50)]
PASS : components::Icon::cleanupTestCase()
Totals: 2 passed, 2 failed, 0 skipped

Revision history for this message
Tim Peeters (tpeeters) wrote :

when I run the unit test locally (trusty, qt 5.2), I get this:

tim@trusty:~/dev/ubuntu-ui-toolkit/r/add-icon-source-property/tests/unit_x11/tst_components$ qmltestrunner -input tst_icon.qml -import ../../../modules
********* Start testing of qmltestrunner *********
Config: Using QtTest library 5.2.1, Qt 5.2.1
PASS : qmltestrunner::Icon::initTestCase()
FAIL! : qmltestrunner::Icon::test_name() Uncaught exception: Invalid write to global property "image"
   Loc: [/home/tim/dev/ubuntu-ui-toolkit/r/add-icon-source-property/tests/unit_x11/tst_components/tst_icon.qml(41)]
FAIL! : qmltestrunner::Icon::test_source() Uncaught exception: Invalid write to global property "image"
   Loc: [/home/tim/dev/ubuntu-ui-toolkit/r/add-icon-source-property/tests/unit_x11/tst_components/tst_icon.qml(49)]
PASS : qmltestrunner::Icon::cleanupTestCase()
Totals: 2 passed, 2 failed, 0 skipped
********* Finished testing of qmltestrunner *********

Revision history for this message
Tim Peeters (tpeeters) :
review: Needs Fixing
Revision history for this message
Tim Peeters (tpeeters) wrote :

to get rid of the errors I get locally, define the variables with var:

            var image = findChild(icon, "image");

after I add that, I get the same failures that jenkins gets.

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

One thing to note... because of how Icon works, order of the tests matters.

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 :

Executing ./tst_components -input tst_icon.qml -maxwarnings 40 -o ../../test_tst_icon.qml.xml,xunitxml -o -,txt
[...]
Totals: 4 passed, 0 failed, 0 skipped
********* Finished testing of components *********
Error: 3 warnings in tst_icon.qml

Huh?

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
Tim Peeters (tpeeters) wrote :

Jenkins tests fail because of warnings. For some reason it looks in the old icon theme?

Config: Using QtTest library 5.3.0, Qt 5.3.0
PASS : components::Icon::initTestCase()
QWARN : components::Icon::test_name() file:///tmp/buildd/ubuntu-ui-toolkit-0.1.49+14.10.20140710bzr1128pkg0utopic429/modules/Ubuntu/Components/Icon11.qml:35:5: QML Image: Failed to get image from provider: image://theme/search
PASS : components::Icon::test_name()
QWARN : components::Icon::test_source() file:///tmp/buildd/ubuntu-ui-toolkit-0.1.49+14.10.20140710bzr1128pkg0utopic429/modules/Ubuntu/Components/Icon11.qml:35:5: QML Image: Failed to get image from provider: image://theme/search
QWARN : components::Icon::test_source() file:///tmp/buildd/ubuntu-ui-toolkit-0.1.49+14.10.20140710bzr1128pkg0utopic429/modules/Ubuntu/Components/Icon11.qml:35:5: QML Image: Invalid image data: file:///usr/share/icons/ubuntu-mobile/status/scalable/search.svg
PASS : components::Icon::test_source()
PASS : components::Icon::cleanupTestCase()
Totals: 4 passed, 0 failed, 0 skipped
********* Finished testing of components *********
Error: 3 warnings in tst_icon.qml
tst_icon.qml exited with 666

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

looks good. Jenkins still fails on the timestamp bug

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

346 + icon.name = "search";
347 + icon.source = "/usr/share/icons/suru/actions/scalable/search.svg";

don't need to set the name here, so remove l. 346

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

> 346 + icon.name = "search";
> 347 + icon.source =
> "/usr/share/icons/suru/actions/scalable/search.svg";
>
> don't need to set the name here, so remove l. 346

I explicitly wanted to check that it's using the source and not the name, it'd be image://theme/search otherwise.

Revision history for this message
Tim Peeters (tpeeters) wrote :

We discussed adding some debugging output to the tests to see where it goes wrong on jenkins. Will you do that in this MR, or somewhere else?

Revision history for this message
Nekhelesh Ramananthan (nik90) wrote :

I manage to reproduce this on all devices using the following sample program,

import QtQuick 2.0
import Ubuntu.Components 0.1

MainView {
    objectName: "mainView"
    applicationName: "com.ubuntu.developer.nik90.IconBug"

    width: units.gu(100)
    height: units.gu(75)

    Page {
        title: i18n.tr("Simple")

        Flickable {
            id: mainFlickable

            anchors.fill: parent
            contentHeight: units.gu(50)
            contentWidth: parent.width

            Icon {
                id: plusIcon
                anchors.centerIn: parent
                color: UbuntuColors.coolGrey
                name: "add"
                height: Math.abs(mainFlickable.contentY)
                width: height
            }
        }
    }
}

So basically the add icon size change as you drag the flickable up an down. I notice the error pop up when the icon is barely visible. Try draggging the flickable up very slowly until the icon becomes really small (looks like a dot) and you should see the message pop up on the console, file:///usr/lib/x86_64-linux-gnu/qt5/qml/Ubuntu/Components/Icon.qml:85:5: QML Image: Failed to get image from provider: image://theme/add

I thought this is happening because the icon size reaches 0 at some point, however on manually setting the icon size to 0, I don't see the error.

Hope this helps.

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

W dniu 28.07.2014 o 16:39, Nekhelesh Ramananthan pisze:
> I thought this is happening because the icon size reaches 0 at some point, however on manually setting the icon size to 0, I don't see the error.

It might be about reaching *almost* 0, meaning it's smaller than 1px, at
which point the QIcon::fromTheme method craps out. Setting to 0
explicitly prevents the image from ever being loaded, so maybe that's
what's happening.

In any case I'm not sure why that would happen in this branch, though...

Revision history for this message
Tim Peeters (tpeeters) wrote :
Revision history for this message
Tim Peeters (tpeeters) wrote :

> W dniu 28.07.2014 o 16:39, Nekhelesh Ramananthan pisze:
> > I thought this is happening because the icon size reaches 0 at some point,
> however on manually setting the icon size to 0, I don't see the error.
>
> It might be about reaching *almost* 0, meaning it's smaller than 1px, at
> which point the QIcon::fromTheme method craps out. Setting to 0
> explicitly prevents the image from ever being loaded, so maybe that's
> what's happening.
>
> In any case I'm not sure why that would happen in this branch, though...

what if units.gu(2) gives some unexpected (small) value on jenkins?

Revision history for this message
Tim Peeters (tpeeters) wrote :

in tst_icon.qml you added cleanup() which sets the icon.name to "".

That changes test_updateIconSize_bug1349769(), which is probably executed after test_name() (alphabetically?) so the name was cleared, and without a name set it may not expose the bug any more when it returns.

Could you add a third icon to the qml app to test setting name and source instead that does not have a name initially?

review: Needs Fixing
Revision history for this message
Tim Peeters (tpeeters) wrote :

thanks for all the changes. Very useful stuff :)

review: Approve
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 :

FYI, the above is for an old revision, fixes checked in and CI should be running now.

Revision history for this message
Tim Peeters (tpeeters) wrote :

tst_icon now needs to use UbuntuTestCase to get findChild(), and the updates in staging for Icon.qml need to go into the new Icon11.qml as well

Revision history for this message
Tim Peeters (tpeeters) :
review: Needs Fixing
Revision history for this message
Tim Peeters (tpeeters) wrote :

thanks

review: Approve
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
PS Jenkins bot (ps-jenkins) :
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Autolanding.
Unapproved changes made after approval.
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-autolanding/299/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-utopic-touch/2792
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic/2230
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-amd64-autolanding/243
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-autolanding/243
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-armhf-autolanding/243/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-utopic-i386-autolanding/243
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/2883
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4035
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-armhf/4035/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/10749
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic/1849
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2489
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/2489/artifact/work/output/*zip*/output.zip

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 'components.api'
2--- components.api 2014-07-29 16:13:32 +0000
3+++ components.api 2014-07-30 23:02:18 +0000
4@@ -92,6 +92,9 @@
5 property string name
6 property color color
7 property color keyColor
8+Icon 1.1
9+Icon10
10+ property url source
11 Label 0.1 1.0
12 Text
13 property string fontSize
14
15=== added file 'modules/Ubuntu/Components/Icon.qdoc'
16--- modules/Ubuntu/Components/Icon.qdoc 1970-01-01 00:00:00 +0000
17+++ modules/Ubuntu/Components/Icon.qdoc 2014-07-30 23:02:18 +0000
18@@ -0,0 +1,87 @@
19+/*
20+ * Copyright (C) 2014 Canonical, Ltd.
21+ *
22+ * This program is free software; you can redistribute it and/or modify
23+ * it under the terms of the GNU General Public License as published by
24+ * the Free Software Foundation; version 3.
25+ *
26+ * This program is distributed in the hope that it will be useful,
27+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
28+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
29+ * GNU General Public License for more details.
30+ *
31+ * You should have received a copy of the GNU General Public License
32+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
33+ */
34+
35+/*!
36+ \qmltype Icon
37+ \inqmlmodule Ubuntu.Components 1.1
38+ \ingroup ubuntu
39+ \brief The Icon component displays an icon from the icon theme.
40+
41+ The icon theme contains a set of standard icons referred to by their name.
42+ Using icons whenever possible enhances consistency accross applications.
43+ Each icon has a name and can have different visual representations depending
44+ on the size requested.
45+
46+ Icons can also be colorized. Setting the \l color property will make all pixels
47+ with the \l keyColor (by default #808080) colored.
48+
49+ Example:
50+ \qml
51+ Icon {
52+ width: 64
53+ height: 64
54+ name: "search"
55+ }
56+ \endqml
57+
58+ Example of colorization:
59+ \qml
60+ Icon {
61+ width: 64
62+ height: 64
63+ name: "search"
64+ color: UbuntuColors.warmGrey
65+ }
66+ \endqml
67+
68+ Icon themes are created following the
69+ \l{http://standards.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html}{Freedesktop Icon Theme Specification}.
70+*/
71+
72+/*!
73+ The name of the icon to display.
74+ \qmlproperty string Icon::name
75+
76+ If both name and source are set, name will be ignored.
77+
78+ \note The complete list of icons available in Ubuntu is not published yet.
79+ For now please refer to the folders where the icon themes are installed:
80+ \list
81+ \li Ubuntu Touch: \l file:/usr/share/icons/suru
82+ \li Ubuntu Desktop: \l file:/usr/share/icons/ubuntu-mono-dark
83+ \endlist
84+ These 2 separate icon themes will be merged soon.
85+*/
86+
87+/*!
88+ The source url of the icon to display. It has precedence over name.
89+
90+ If both name and source are set, name will be ignored.
91+
92+ \since Ubuntu.Components 1.1
93+ \qmlproperty url Icon::source
94+*/
95+
96+/*!
97+ The color that all pixels that originally are of color \l keyColor should take.
98+ \qmlproperty color Icon::color
99+*/
100+
101+/*!
102+ The color of the pixels that should be colorized.
103+ By default it is set to #808080.
104+ \qmlproperty color Icon::keyColor
105+*/
106
107=== renamed file 'modules/Ubuntu/Components/Icon.qml' => 'modules/Ubuntu/Components/Icon10.qml'
108--- modules/Ubuntu/Components/Icon.qml 2014-07-30 11:39:21 +0000
109+++ modules/Ubuntu/Components/Icon10.qml 2014-07-30 23:02:18 +0000
110@@ -1,5 +1,5 @@
111 /*
112- * Copyright (C) 2013 Canonical, Ltd.
113+ * Copyright (C) 2014 Canonical, Ltd.
114 *
115 * This program is free software; you can redistribute it and/or modify
116 * it under the terms of the GNU General Public License as published by
117@@ -16,74 +16,24 @@
118
119 import QtQuick 2.0
120
121-/*!
122- \qmltype Icon
123- \inqmlmodule Ubuntu.Components 1.1
124- \ingroup ubuntu
125- \brief The Icon component displays an icon from the icon theme.
126-
127- The icon theme contains a set of standard icons referred to by their name.
128- Using icons whenever possible enhances consistency accross applications.
129- Each icon has a name and can have different visual representations depending
130- on the size requested.
131-
132- Icons can also be colorized. Setting the \l color property will make all pixels
133- with the \l keyColor (by default #808080) colored.
134-
135- Example:
136- \qml
137- Icon {
138- width: 64
139- height: 64
140- name: "search"
141- }
142- \endqml
143-
144- Example of colorization:
145- \qml
146- Icon {
147- width: 64
148- height: 64
149- name: "search"
150- color: UbuntuColors.warmGrey
151- }
152- \endqml
153-
154- Icon themes are created following the
155- \l{http://standards.freedesktop.org/icon-theme-spec/icon-theme-spec-latest.html}{Freedesktop Icon Theme Specification}.
156-*/
157 Item {
158 id: icon
159
160- /*!
161- The name of the icon to display.
162- \qmlproperty string name
163-
164- \note The complete list of icons available in Ubuntu is not published yet.
165- For now please refer to the folders where the icon themes are installed:
166- \list
167- \li Ubuntu Touch: \l file:/usr/share/icons/suru
168- \li Ubuntu Desktop: \l file:/usr/share/icons/ubuntu-mono-dark
169- \endlist
170- These 2 separate icon themes will be merged soon.
171- */
172 property string name
173
174 /*!
175- The color that all pixels that originally are of color \l keyColor should take.
176 \qmlproperty color color
177 */
178 property alias color: colorizedImage.keyColorOut
179
180 /*!
181- The color of the pixels that should be colorized.
182- By default it is set to #808080.
183 \qmlproperty color keyColor
184 */
185 property alias keyColor: colorizedImage.keyColorIn
186
187 Image {
188 id: image
189+ objectName: "image"
190 anchors.fill: parent
191
192 /* Necessary so that icons are not loaded before a size is set. */
193@@ -99,6 +49,7 @@
194 Connections {
195 target: icon
196 onNameChanged: image.update()
197+ onSourceChanged: image.update()
198 }
199
200 function update() {
201@@ -107,7 +58,11 @@
202 if (width > 0 && height > 0 && icon.name) {
203 sourceSize.width = width;
204 sourceSize.height = height;
205- source = "image://theme/%1".arg(icon.name);
206+ if (icon.hasOwnProperty("source")) {
207+ source = icon.source;
208+ } else {
209+ source = "image://theme/%1".arg(icon.name);
210+ }
211 } else {
212 source = "";
213 sourceSize.width = 0;
214
215=== added file 'modules/Ubuntu/Components/Icon11.qml'
216--- modules/Ubuntu/Components/Icon11.qml 1970-01-01 00:00:00 +0000
217+++ modules/Ubuntu/Components/Icon11.qml 2014-07-30 23:02:18 +0000
218@@ -0,0 +1,21 @@
219+/*
220+ * Copyright (C) 2014 Canonical, Ltd.
221+ *
222+ * This program is free software; you can redistribute it and/or modify
223+ * it under the terms of the GNU General Public License as published by
224+ * the Free Software Foundation; version 3.
225+ *
226+ * This program is distributed in the hope that it will be useful,
227+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
228+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
229+ * GNU General Public License for more details.
230+ *
231+ * You should have received a copy of the GNU General Public License
232+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
233+ */
234+
235+import QtQuick 2.0
236+
237+Icon10 {
238+ property url source: name ? "image://theme/%1".arg(name) : ""
239+}
240
241=== modified file 'modules/Ubuntu/Components/qmldir'
242--- modules/Ubuntu/Components/qmldir 2014-07-29 16:13:32 +0000
243+++ modules/Ubuntu/Components/qmldir 2014-07-30 23:02:18 +0000
244@@ -35,7 +35,7 @@
245 internal PageWrapper PageWrapper.qml
246 UbuntuShape 0.1 UbuntuShape.qml
247 CrossFadeImage 0.1 CrossFadeImage10.qml
248-Icon 0.1 Icon.qml
249+Icon 0.1 Icon10.qml
250 OrientationHelper 0.1 OrientationHelper.qml
251 MathUtils 0.1 mathUtils.js
252 SliderUtils 0.1 sliderUtils.js
253@@ -80,7 +80,7 @@
254 Header 1.0 Header.qml
255 UbuntuShape 1.0 UbuntuShape.qml
256 CrossFadeImage 1.0 CrossFadeImage10.qml
257-Icon 1.0 Icon.qml
258+Icon 1.0 Icon10.qml
259 OrientationHelper 1.0 OrientationHelper.qml
260 MathUtils 1.0 mathUtils.js
261 SliderUtils 1.0 sliderUtils.js
262@@ -103,3 +103,4 @@
263 PageHeadConfiguration 1.1 PageHeadConfiguration.qml
264 PageHeadSections 1.1 PageHeadSections.qml
265 PageHeadState 1.1 PageHeadState.qml
266+Icon 1.1 Icon11.qml
267
268=== added file 'tests/unit_x11/tst_components/tst_icon.qml'
269--- tests/unit_x11/tst_components/tst_icon.qml 1970-01-01 00:00:00 +0000
270+++ tests/unit_x11/tst_components/tst_icon.qml 2014-07-30 23:02:18 +0000
271@@ -0,0 +1,89 @@
272+/*
273+ * Copyright 2012 Canonical Ltd.
274+ *
275+ * This program is free software; you can redistribute it and/or modify
276+ * it under the terms of the GNU Lesser General Public License as published by
277+ * the Free Software Foundation; version 3.
278+ *
279+ * This program is distributed in the hope that it will be useful,
280+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
281+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
282+ * GNU Lesser General Public License for more details.
283+ *
284+ * You should have received a copy of the GNU Lesser General Public License
285+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
286+ */
287+
288+import QtQuick 2.0
289+import QtTest 1.0
290+import Ubuntu.Components 1.1
291+import Ubuntu.Test 1.0
292+
293+Item {
294+ width: units.gu(50)
295+ height: units.gu(50)
296+
297+ Row {
298+ anchors.centerIn: parent
299+ height: units.gu(10)
300+ width: childrenRect.width
301+
302+ Icon {
303+ // Fails to load the icon when suru-icon-theme or libqt5svg5 are
304+ // not installed. This causes a warning and rejection
305+ // by jenkins continuous integration.
306+ name: "add"
307+ height: parent.height
308+ width: height
309+ }
310+ Icon {
311+ // Fails when the icon is becoming invisible when non-atomic updates
312+ // cause sourceSize.width or sourceSize.height to be 0 before other
313+ // properties are updated.
314+ id: icon
315+ width: visible ? units.gu(10) : 0
316+ height: width
317+ name: "search"
318+ }
319+ Icon {
320+ id: icon2
321+ width: units.gu(10)
322+ height: width
323+ }
324+ }
325+
326+ UbuntuTestCase {
327+ name: "Icon"
328+ when: windowShown
329+
330+ function cleanup() {
331+ icon2.name = "";
332+ }
333+
334+ function test_updateIconSize_bug1349769() {
335+ icon.visible = false;
336+ // causes "QML Image: Failed to get image from provider: image://theme/search"
337+ // warning when sourceSize.width or sourceSize.height becomes 0 while
338+ // while still trying to render the icon. Tests will pass with the warning, but
339+ // the MR is rejected by jenkins continuous integration.
340+ }
341+
342+ function test_name() {
343+ icon2.name = "search";
344+
345+ var image = findChild(icon2, "image");
346+ compare(image.source, "image://theme/search",
347+ "Source of the image should be image://theme/{name}.");
348+ }
349+
350+ function test_source() {
351+ icon2.name = "search";
352+ icon2.source = "/usr/share/icons/suru/actions/scalable/search.svg";
353+
354+ var image = findChild(icon2, "image");
355+ compare(image.source,
356+ "file:///usr/share/icons/suru/actions/scalable/search.svg",
357+ "Source of the image should equal icon2.source.");
358+ }
359+ }
360+}
361
362=== removed file 'tests/unit_x11/tst_components/tst_icon.qml'
363--- tests/unit_x11/tst_components/tst_icon.qml 2014-07-30 12:11:43 +0000
364+++ tests/unit_x11/tst_components/tst_icon.qml 1970-01-01 00:00:00 +0000
365@@ -1,61 +0,0 @@
366-/*
367- * Copyright 2012 Canonical Ltd.
368- *
369- * This program is free software; you can redistribute it and/or modify
370- * it under the terms of the GNU Lesser General Public License as published by
371- * the Free Software Foundation; version 3.
372- *
373- * This program is distributed in the hope that it will be useful,
374- * but WITHOUT ANY WARRANTY; without even the implied warranty of
375- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
376- * GNU Lesser General Public License for more details.
377- *
378- * You should have received a copy of the GNU Lesser General Public License
379- * along with this program. If not, see <http://www.gnu.org/licenses/>.
380- */
381-
382-import QtQuick 2.0
383-import QtTest 1.0
384-import Ubuntu.Components 1.1
385-
386-Item {
387- width: units.gu(50)
388- height: units.gu(50)
389-
390- Row {
391- anchors.centerIn: parent
392- height: units.gu(10)
393- width: childrenRect.width
394-
395- Icon {
396- // Fails to load the icon when suru-icon-theme or libqt5svg5 are
397- // not installed. This causes a warning and rejection
398- // by jenkins continuous integration.
399- name: "add"
400- height: parent.height
401- width: height
402- }
403- Icon {
404- // Fails when the icon is becoming invisible when non-atomic updates
405- // cause sourceSize.width or sourceSize.height to be 0 before other
406- // properties are updated.
407- id: icon
408- width: visible ? units.gu(10) : 0
409- height: width
410- name: "search"
411- }
412- }
413-
414- TestCase {
415- name: "Icon"
416- when: windowShown
417-
418- function test_updateIconSize_bug1349769() {
419- icon.visible = false;
420- // causes "QML Image: Failed to get image from provider: image://theme/search"
421- // warning when sourceSize.width or sourceSize.height becomes 0 while
422- // while still trying to render the icon. Tests will pass with the warning, but
423- // the MR is rejected by jenkins continuous integration.
424- }
425- }
426-}

Subscribers

People subscribed via source and target branches