Ubuntu UI Toolkit

Merge lp:~tpeeters/ubuntu-ui-toolkit/icon-disabled into lp:ubuntu-ui-toolkit/staging

  1. icon-disabled
  2. Merge into staging
Proposed by Tim Peeters on 2016-03-16
Status: Rejected
Rejected by: Tim Peeters on 2016-03-23
Proposed branch: lp:~tpeeters/ubuntu-ui-toolkit/icon-disabled
Merge into: lp:ubuntu-ui-toolkit/staging
Prerequisite: lp:~tpeeters/ubuntu-ui-toolkit/gallery-export-fix
Diff against target: 113 lines (+45/-3)
3 files modified
examples/ubuntu-ui-toolkit-gallery/Icons.qml (+30/-2)
src/Ubuntu/Components/1.3/Icon.qml (+1/-0)
tests/unit_x11/tst_components/tst_icon13.qml (+14/-1)
To merge this branch: bzr merge lp:~tpeeters/ubuntu-ui-toolkit/icon-disabled
Reviewer Review Type Date Requested Status
Tim Peeters Disapprove on 2016-03-23
ubuntu-sdk-build-bot continuous-integration Needs Fixing on 2016-03-23
Christian Dywan 2016-03-16 Needs Fixing on 2016-03-16
Review via email: mp+289308@code.launchpad.net

Commit Message

Set the opacity of disabled icons to 0.3.

To post a comment you must log in.
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Approve (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :

PASSED: Continuous integration, rev:1903
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/170/
Executed test runs:
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/520
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/531
        deb: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/531/artifact/artifacts/*zip*/artifacts.zip
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/300
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/274
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1886/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1882
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1885
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1883
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/1884

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/170/rebuild

review: Approve (continuous-integration)
Christian Dywan (kalikiana) wrote :

Please add a unit test. It might be as simple as checking that the opacity is not 1 when enabled is false.

review: Needs Fixing
Tim Peeters (tpeeters) wrote :

We stopped adding 'silly' unit tests that check that components have the exact color that we define in the component or its style because they only cause overhead when changing a color, and don't catch any real bugs. I consider a unit test to check the opacity to be similarly useless.

Tim Peeters (tpeeters) wrote :

After discussing, we decided that I will add a test to verify that the opacity is reduced for a disabled icon.

lp:~tpeeters/ubuntu-ui-toolkit/icon-disabled updated on 2016-03-23
1904. By Tim Peeters on 2016-03-23

sync trunk

1905. By Tim Peeters on 2016-03-23

add unit test

ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :
review: Needs Fixing (continuous-integration)
ubuntu-sdk-build-bot (ubuntu-sdk-build-bot) wrote :

FAILED: Continuous integration, rev:1905
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/196/
Executed test runs:
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-stable/555/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-armhf-stable/567/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-amd64-devel/335/console
    FAILURE: https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci-i386-gles-stable/309/console
    None: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/2061/console
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/2056
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/2054
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/2059
    SUCCESS: https://jenkins.ubuntu.com/ubuntu-sdk/job/generic-update-mp/2060

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/ubuntu-sdk/job/ubuntu-ui-toolkit-ci/196/rebuild

review: Needs Fixing (continuous-integration)
Tim Peeters (tpeeters) wrote :

Fixed with the new palette. See the discussion on https://bugs.launchpad.net/ubuntu-ux/+bug/1393485

For the header, the palette takes care of it. If other components need to update their opacity, probably they need to use the new palette properly, or set the opacity in the component itself while the Icon keeps its default opacity of 1.

review: Disapprove

Unmerged revisions

1905. By Tim Peeters on 2016-03-23

add unit test

1904. By Tim Peeters on 2016-03-23

sync trunk

1903. By Tim Peeters on 2016-03-16

set icon opacity to 0.3 when disabled

1902. By Tim Peeters on 2016-03-16

sync gallery-export-fix

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/ubuntu-ui-toolkit-gallery/Icons.qml'
2--- examples/ubuntu-ui-toolkit-gallery/Icons.qml 2015-08-06 10:26:25 +0000
3+++ examples/ubuntu-ui-toolkit-gallery/Icons.qml 2016-03-23 17:23:36 +0000
4@@ -1,5 +1,5 @@
5 /*
6- * Copyright 2013 Canonical Ltd.
7+ * Copyright 2016 Canonical Ltd.
8 *
9 * This program is free software; you can redistribute it and/or modify
10 * it under the terms of the GNU Lesser General Public License as published by
11@@ -14,7 +14,7 @@
12 * along with this program. If not, see <http://www.gnu.org/licenses/>.
13 */
14
15-import QtQuick 2.0
16+import QtQuick 2.4
17 import Ubuntu.Components 1.3
18 import Ubuntu.Components.Popups 1.3
19 import Qt.labs.folderlistmodel 2.1
20@@ -76,6 +76,34 @@
21 }
22
23 TemplateRow {
24+ title: i18n.tr("Disabled")
25+ spacing: units.gu(2)
26+
27+ Icon {
28+ name: "stock_alarm-clock"
29+ width: units.gu(3)
30+ height: width
31+ enabled: false
32+ }
33+
34+ Icon {
35+ name: "stock_alarm-clock"
36+ color: UbuntuColors.orange
37+ width: units.gu(3)
38+ height: width
39+ enabled: false
40+ }
41+
42+ Icon {
43+ name: "stock_alarm-clock"
44+ color: UbuntuColors.lightAubergine
45+ width: units.gu(3)
46+ height: width
47+ enabled: false
48+ }
49+ }
50+
51+ TemplateRow {
52 title: i18n.tr("Theme")
53 spacing: units.gu(2)
54 height: iconFlow.height
55
56=== modified file 'src/Ubuntu/Components/1.3/Icon.qml'
57--- src/Ubuntu/Components/1.3/Icon.qml 2016-02-11 08:56:13 +0000
58+++ src/Ubuntu/Components/1.3/Icon.qml 2016-03-23 17:23:36 +0000
59@@ -59,6 +59,7 @@
60
61 Item {
62 id: icon
63+ opacity: enabled ? 1.0 : 0.3
64
65 /*!
66 The name of the icon to display.
67
68=== modified file 'tests/unit_x11/tst_components/tst_icon13.qml'
69--- tests/unit_x11/tst_components/tst_icon13.qml 2016-03-15 13:26:55 +0000
70+++ tests/unit_x11/tst_components/tst_icon13.qml 2016-03-23 17:23:36 +0000
71@@ -20,7 +20,7 @@
72 import Ubuntu.Test 1.3
73
74 Item {
75- width: units.gu(50)
76+ width: units.gu(60)
77 height: units.gu(50)
78
79 Row {
80@@ -29,6 +29,7 @@
81 width: childrenRect.width
82
83 Icon {
84+ id: icon0
85 // Fails to load the icon when suru-icon-theme or libqt5svg5 are
86 // not installed. This causes a warning and rejection
87 // by jenkins continuous integration.
88@@ -58,6 +59,13 @@
89 height: width
90 source: Qt.resolvedUrl("tst_icon-select.png")
91 }
92+ Icon {
93+ id: disabledIcon
94+ enabled: false
95+ width: units.gu(10)
96+ height: width
97+ name: "account"
98+ }
99 }
100
101 UbuntuTestCase {
102@@ -103,6 +111,11 @@
103 "Source of the image should equal icon2.source.");
104 }
105
106+ function test_opacity_bug1393485() {
107+ compare(icon0.opacity, 1.0, "Enabled icon does not have opacity of 1.");
108+ verify(disabledIcon.opacity < 0.2, "Disabled icon is not semi-transparent.");
109+ }
110+
111 function test_keyColor() {
112 icon.visible = true;
113 var image = findChild(icon, "image");

Subscribers

People subscribed via source and target branches