Merge lp:~lukas-kde/unity8/fixNotificationsColorWithNewUitk into lp:unity8

[Lukáš Tinkl]

Fix notifications with the new UITK color schemes

Fix notifications with the new UITK color schemes

Try to follow the new SDK palette as closely as possible

* 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

* Did you make sure that your branch does not contain spurious tags?

Yes

* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?

Yes

* If you changed the UI, has there been a design review?

No (altho this patch tries to follow the SDK Palette as closely as possible)

[Unity8 CI Bot (unity8-ci-bot)]

PASSED: Continuous integration, rev:2277
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/697/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/397
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial,testname=qmluitests.sh/397
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=autopilot.sh/397
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/918
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/934
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/934
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/932
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/932/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/932
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/932/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/932
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/932/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/932
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/932/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/932
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/932/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/932
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/932/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/697/rebuild

review: Approve (continuous-integration)

[Daniel d'Andrada (dandrader)]

"""
- readonly property color red: "#fc4949"
- readonly property color green: "#3fb24f"
+ readonly property color red: UbuntuColors.red
+ readonly property color green: UbuntuColors.green
"""

Looks like those red and green properties are now redundant. You could refer directly to UbuntuColors.red and UbuntuColors.green and the code will be clear just the same.

[Michael Terry (mterry)]

Agreed with Daniel.

And what's with the continued use of sdLightGrey and the manually-specified rgba value? Can we replace that with a theme equivalent?

Otherwise, works fine in my brief testing on the phone, and playing with the tryNotifications window (which, if we had more time, I would ask you to expand to include more scenarios, like a light/dark mode toggle).

review: Needs Fixing

[lp:~lukas-kde/unity8/fixNotificationsColorWithNewUitk]

2278. By Lukáš Tinkl on 2016-03-14

remove useless aliases for builtin colors

[Lukáš Tinkl (lukas-kde)]

> Agreed with Daniel.
>
> And what's with the continued use of sdLightGrey and the manually-specified
> rgba value? Can we replace that with a theme equivalent?

Yeah, definitely something for the future (most probably together with designers at the upcoming sprint)...

> Otherwise, works fine in my brief testing on the phone, and playing with the
> tryNotifications window (which, if we had more time, I would ask you to expand
> to include more scenarios, like a light/dark mode toggle).

.... and this too

[Lukáš Tinkl (lukas-kde)]

That said, I removed the hardcoded aliases

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2278
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/734/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/411
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial,testname=qmluitests.sh/411
    FAILURE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=autopilot.sh/411/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/963
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/979
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/979
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/977
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/977/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/977
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/977/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/977
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/977/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/977
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/977/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/977
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/977/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/977
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/977/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/734/rebuild

review: Needs Fixing (continuous-integration)

[lp:~lukas-kde/unity8/fixNotificationsColorWithNewUitk]

2279. By Lukáš Tinkl on 2016-03-15

fix the broken binding (reaching out of context)

for inverseMode and make it propertly exported, so that it can
be tested

[Unity8 CI Bot (unity8-ci-bot)]

FAILED: Continuous integration, rev:2279
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/751/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/417
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial,testname=qmluitests.sh/417
    FAILURE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=phone-armhf,release=vivid+overlay,testname=autopilot.sh/417/console
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/988
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1004
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial/1004
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1002
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1002/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1002
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial/1002/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1002
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1002/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1002
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial/1002/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1002
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1002/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1002
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial/1002/artifact/output/*zip*/output.zip

Click here to trigger a rebuild:
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/751/rebuild

review: Needs Fixing (continuous-integration)

[Michael Terry (mterry)]

 * Did you perform an exploratory manual test run of the code change and any related functionality?
 Yes, everything I tested seemed fine.

 * Did CI run pass? If not, please explain why.
 No, some weird autopilot env failure

 * Did you make sure that the branch does not contain spurious tags?
 Yes

It would be nice if we could do something more dynamic for sdLightGrey, but Lukas says there isn't a good replacement in the theme yet.

review: Approve

=== modified file 'qml/Notifications/Notification.qml'
--- qml/Notifications/Notification.qml	2016-03-10 22:41:22 +0000
+++ qml/Notifications/Notification.qml	2016-03-15 20:18:12 +0000
@@ -41,15 +41,12 @@
     property int maxHeight
     property int margins
     readonly property bool draggable: (type === Notification.SnapDecision && state === "contracted") || type === Notification.Interactive || type === Notification.Ephemeral
-    readonly property bool darkOnBright: panel.indicators.shown || type === Notification.SnapDecision
-    readonly property color red: "#fc4949"
-    readonly property color green: "#3fb24f"
+    readonly property bool darkOnBright: inverseMode || type === Notification.SnapDecision
     readonly property color sdLightGrey: "#eaeaea"
-    readonly property color sdDarkGrey: "#dddddd"
-    readonly property color sdFontColor: "#5d5d5d"
     readonly property real contentSpacing: units.gu(2)
     readonly property bool canBeClosed: type === Notification.Ephemeral
     property bool hasMouse
+    property bool inverseMode
     property url background: ""
 
     objectName: "background"
@@ -59,7 +56,7 @@
     opacity: 1 - (x / notification.width) // FIXME: non-zero initially because of LP: #1354406 workaround, we want this to start at 0 upon creation eventually
 
     theme: ThemeSettings {
-        name: "Ubuntu.Components.Themes.Ambiance"
+        name: darkOnBright ? "Ubuntu.Components.Themes.Ambiance" : "Ubuntu.Components.Themes.SuruDark"
     }
 
     state: {
@@ -285,7 +282,6 @@
                         }
                         visible: type !== Notification.Confirmation
                         fontSize: "medium"
-                        color: darkOnBright ? sdFontColor : theme.palette.normal.backgroundText
                         elide: Text.ElideRight
                         textFormat: Text.PlainText
                     }
@@ -300,7 +296,6 @@
                         }
                         visible: body != "" && type !== Notification.Confirmation
                         fontSize: "small"
-                        color: darkOnBright ? sdFontColor : theme.palette.normal.backgroundText
                         wrapMode: Text.Wrap
                         maximumLineCount: type == Notification.SnapDecision ? 12 : 2
                         elide: Text.ElideRight
@@ -341,7 +336,6 @@
                 anchors.horizontalCenter: parent.horizontalCenter
                 visible: type === Notification.Confirmation && body !== ""
                 fontSize: "medium"
-                color: darkOnBright ? sdFontColor : theme.palette.normal.backgroundText
                 wrapMode: Text.WordWrap
                 maximumLineCount: 1
                 elide: Text.ElideRight
@@ -361,7 +355,7 @@
                 }
 
                 height: units.gu(1)
-                backgroundColor: darkOnBright ? UbuntuColors.darkGrey : UbuntuColors.lightGrey
+                backgroundColor: theme.palette.normal.background
                 aspect: UbuntuShape.Flat
                 radius: "small"
 
@@ -370,7 +364,7 @@
                     objectName: "innerBar"
                     width: valueIndicator.width * valueIndicator.value / 100
                     height: units.gu(1)
-                    backgroundColor: notification.hints["x-canonical-value-bar-tint"] === "true" ? UbuntuColors.orange : darkOnBright ? UbuntuColors.lightGrey : "white"
+                    backgroundColor: notification.hints["x-canonical-value-bar-tint"] === "true" ? theme.palette.normal.activity : theme.palette.highlighted.foreground
                     aspect: UbuntuShape.Flat
                     radius: "small"
                 }
@@ -447,7 +441,7 @@
                                 objectName: "notify_oot_button" + index
                                 width: oneOverTwoCase.width
                                 text: oneOverTwoLoaderTop.actionLabel
-                                color: notification.hints["x-canonical-private-affirmative-tint"] == "true" ? green : sdDarkGrey
+                                color: notification.hints["x-canonical-private-affirmative-tint"] == "true" ? UbuntuColors.green : theme.palette.normal.baseText
                                 onClicked: notification.notification.invokeAction(oneOverTwoLoaderTop.actionId)
                             }
                         }
@@ -475,7 +469,7 @@
                                     objectName: "notify_oot_button" + index
                                     width: oneOverTwoCase.width / 2 - spacing * 2
                                     text: oneOverTwoLoaderBottom.actionLabel
-                                    color: index == 1 && notification.hints["x-canonical-private-rejection-tint"] == "true" ? red : sdDarkGrey
+                                    color: index == 1 && notification.hints["x-canonical-private-rejection-tint"] == "true" ? UbuntuColors.red : theme.palette.normal.baseText
                                     onClicked: notification.notification.invokeAction(oneOverTwoLoaderBottom.actionId)
                                 }
                             }
@@ -536,12 +530,12 @@
                                 width: buttonRow.width / 2 - spacing * 2
                                 text: loader.actionLabel
                                 color: {
-                                    var result = sdDarkGrey;
+                                    var result = theme.palette.normal.baseText;
                                     if (index == 0 && notification.hints["x-canonical-private-affirmative-tint"] == "true") {
-                                        result = green;
+                                        result = UbuntuColors.green;
                                     }
                                     if (index == 1 && notification.hints["x-canonical-private-rejection-tint"] == "true") {
-                                        result = red;
+                                        result = UbuntuColors.red;
                                     }
                                     return result;
                                 }

=== modified file 'qml/Notifications/Notifications.qml'
--- qml/Notifications/Notifications.qml	2015-10-26 09:59:50 +0000
+++ qml/Notifications/Notifications.qml	2016-03-15 20:18:12 +0000
@@ -29,6 +29,7 @@
     property real margin
     property bool useModal: snapDecisionProxyModel.count > 0
     property bool hasMouse
+    property bool inverseMode
     property url background: ""
 
     UnitySortFilterProxyModel {
@@ -61,6 +62,7 @@
         maxHeight: notificationList.height
         margins: notificationList.margin
         hasMouse: notificationList.hasMouse
+        inverseMode: notificationList.inverseMode
         background: notificationList.background
 
         // make sure there's no opacity-difference between the several

=== modified file 'qml/Notifications/OptionToggle.qml'
--- qml/Notifications/OptionToggle.qml	2015-11-04 14:57:13 +0000
+++ qml/Notifications/OptionToggle.qml	2016-03-15 20:18:12 +0000
@@ -96,7 +96,7 @@
 
                             width: parent.width
                             text: splitLabel[3]
-                            color:"#5d5d5d"
+                            color: theme.palette.normal.backgroundText
                             fontSize: "small"
                             maximumLineCount: 1
                             elide: Text.ElideRight

=== modified file 'qml/Shell.qml'
--- qml/Shell.qml	2016-03-10 22:37:44 +0000
+++ qml/Shell.qml	2016-03-15 20:18:12 +0000
@@ -663,6 +663,7 @@
             model: NotificationBackend.Model
             margin: units.gu(1)
             hasMouse: shell.hasMouse
+            inverseMode: panel.indicators.shown
             background: wallpaperResolver.background
 
             y: topmostIsFullscreen ? 0 : panel.panelHeight

=== modified file 'tests/qmltests/Notifications/tst_Notifications.qml'
--- tests/qmltests/Notifications/tst_Notifications.qml	2015-11-23 15:41:34 +0000
+++ tests/qmltests/Notifications/tst_Notifications.qml	2016-03-15 20:18:12 +0000
@@ -187,7 +187,8 @@
 
                 anchors.fill: parent
                 model: mockModel
-                hasMouse: false
+                hasMouse: fakeMouseCB.checked
+                inverseMode: inverseModeCB.checked
             }
         }
 
@@ -261,19 +262,23 @@
                     Layout.fillWidth: true
                     CheckBox {
                         id: fakeMouseCB
-                        onClicked: {
-                            if (checked) {
-                                notifications.hasMouse = true;
-                            } else {
-                                notifications.hasMouse = false;
-                            }
-                        }
                     }
                     Label {
                         text: "With fake mouse"
                         anchors.verticalCenter: parent.verticalCenter
                     }
                 }
+
+                RowLayout {
+                    Layout.fillWidth: true
+                    CheckBox {
+                        id: inverseModeCB
+                    }
+                    Label {
+                        text: "Inverse mode"
+                        anchors.verticalCenter: parent.verticalCenter
+                    }
+                }
             }
         }