Merge lp:~lukas-kde/unity8/fullscreenNotifications-lp1581498 into lp:unity8

[Lukáš Tinkl]

Fullscreen notification bug fixes

Fullscreen notification bug fixes

- don't let them be pushed down by other snap decisions (e.g. wifi dialog, USB debugging prompt); set their priority higher
- don't allow swiping them away, they must be acted upon
- perform the "reject" action unconditionally (fixes a discrepancy between the mock and the real model)

Fixes lp:1581498

[lp:~lukas-kde/unity8/fullscreenNotifications-lp1581498]

2400. By Lukáš Tinkl on 2016-05-16

perform the "reject" action unconditionally

the real model doesn't have a "count" property, unlike the mock model, grmbl...

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

FAILED: Continuous integration, rev:2399
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1222/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/759
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/759
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1639
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1592
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1592
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1585/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1585
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1585/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

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

PASSED: Continuous integration, rev:2400
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1225/
Executed test runs:
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/762
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/762
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1642
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1595
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1595
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1588
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1588/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1588
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1588/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1588
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1588/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1588
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1588/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1588
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1588/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1588
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1588/artifact/output/*zip*/output.zip

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

review: Approve (continuous-integration)

[Albert Astals Cid (aacid)]

did you base qml/Notifications/Notifications.qml on my branch or did you arrive to the same solution for the same problem independently?

review: Needs Information

[Lukáš Tinkl (lukas-kde)]

I partly based it on yours as I needed it here as well and because launchpad can have only one prereq...

[Michael Zanetti (mzanetti)]

nope, not fixed yet: http://i.imgur.com/kIbEZwc.png

review: Needs Fixing

[Lukáš Tinkl (lukas-kde)]

> nope, not fixed yet: http://i.imgur.com/kIbEZwc.png

That one is special, it's a design decision I think; the model inserts the "confirmations" (e.g. volume dialog) unconditionally at the top of the visible ones.

[Albert Astals Cid (aacid)]

will abstain since mzanetti seems to be taking care of the review.

review: Abstain

[Michael Zanetti (mzanetti)]

well, can also reproduce it with wifi ones still:

http://i.imgur.com/PxtbFSL.png

review: Needs Fixing

[lp:~lukas-kde/unity8/fullscreenNotifications-lp1581498]

2401. By Lukáš Tinkl on 2016-05-18

better fix for handling the fullscreen state

[Lukáš Tinkl (lukas-kde)]

Alright, hopefully better now - can't reproduce with neither wifi dialog or the USB debugging prompt

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

FAILED: Continuous integration, rev:2401
https://unity8-jenkins.ubuntu.com/job/lp-unity8-ci/1240/
Executed test runs:
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=vivid+overlay,testname=qmluitests.sh/775
    UNSTABLE: https://unity8-jenkins.ubuntu.com/job/test-0-autopkgtest/label=amd64,release=xenial+overlay,testname=qmluitests.sh/775
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-0-fetch/1672
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=vivid+overlay/1622
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-1-sourcepkg/release=xenial+overlay/1622
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1615
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=vivid+overlay/1615/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1615
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=amd64,release=xenial+overlay/1615/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1615
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=vivid+overlay/1615/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1615
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=armhf,release=xenial+overlay/1615/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1615
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=vivid+overlay/1615/artifact/output/*zip*/output.zip
    SUCCESS: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1615
        deb: https://unity8-jenkins.ubuntu.com/job/build-2-binpkg/arch=i386,release=xenial+overlay/1615/artifact/output/*zip*/output.zip

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

review: Needs Fixing (continuous-integration)

[Michael Zanetti (mzanetti)]

I can still reproduce it with the WiFi dialog... I think that's a bit of a constructed use case though... The USB port popup seems definitely fixed now.

[Michael Zanetti (mzanetti)]

Ok... on more attempts I can't reproduce it with the wifi one any more either. There might be a race somewhere but oh well, probably not worth trying to find that as it will most likely require to refactor the whole thing and there's a notification rework showing up on the horizon anyways.

I'm ok with the code changes. It looks like duct-taping the thing, but not sure I'd come up with something better without ending up at the complete refactor again.

Let's get this in then.

* Tested, ok

* Ci tests did not pass, however, looks really unrelated

review: Approve

=== modified file 'qml/Notifications/Notification.qml'
--- qml/Notifications/Notification.qml	2016-05-18 18:14:47 +0000
+++ qml/Notifications/Notification.qml	2016-05-18 18:14:47 +0000
@@ -55,11 +55,13 @@
         name: "Ubuntu.Components.Themes.Ambiance"
     }
 
+    signal dismissed()
+
     readonly property bool expanded: {
         var result = false;
 
         if (type === Notification.SnapDecision) {
-            if (ListView.view.currentIndex === index) {
+            if (ListView.view.currentIndex === index || fullscreen) {
                 result = true;
             } else {
                 if (ListView.view.count > 2) {
@@ -103,10 +105,11 @@
             ListView.view.currentIndex = -1;
         }
 
-        if (notification.actions.count > 1) { // perform the "reject" action
-            notification.notification.invokeAction(notification.actions.data(1, ActionModel.RoleActionId));
-        }
+        // perform the "reject" action
+        notification.notification.invokeAction(notification.actions.data(1, ActionModel.RoleActionId));
+
         notification.notification.close();
+        notification.dismissed()
     }
 
     Behavior on x {
@@ -119,6 +122,12 @@
         }
     }
 
+    onFullscreenChanged: {
+        if (fullscreen) {
+            notification.notification.urgency = Notification.Critical;
+        }
+    }
+
     Behavior on height {
         UbuntuNumberAnimation {
             duration: UbuntuAnimation.SnapDuration
@@ -192,6 +201,7 @@
             onNameOwnerChanged: {
                 if (lastNameOwner !== "" && nameOwner === "" && notification.notification !== undefined) {
                     notification.notification.close()
+                    notification.dismissed()
                 }
                 lastNameOwner = nameOwner
             }
@@ -203,7 +213,7 @@
             anchors.fill: parent
             objectName: "interactiveArea"
 
-            drag.target: notification
+            drag.target: !fullscreen ? notification : undefined
             drag.axis: Drag.XAxis
             drag.minimumX: -notification.width
             drag.maximumX: notification.width
@@ -386,7 +396,7 @@
                 objectName: "dialogListView"
                 spacing: notification.margins
 
-                visible: count > 0 && notification.expanded
+                visible: count > 0 && (notification.expanded || notification.fullscreen)
 
                 anchors {
                     left: parent.left
@@ -417,6 +427,7 @@
                         }
                         onAccepted: {
                             notification.notification.invokeAction(actionRepeater.itemAt(0).actionId)
+                            notification.dismissed()
                         }
                     }
                 }

=== modified file 'qml/Notifications/NotificationMenuItemFactory.qml'
--- qml/Notifications/NotificationMenuItemFactory.qml	2016-04-05 19:29:33 +0000
+++ qml/Notifications/NotificationMenuItemFactory.qml	2016-05-18 18:14:47 +0000
@@ -150,15 +150,18 @@
             onEntered: {
                 menuModel.changeState(menuIndex, passphrase);
                 clear(false);
+                notification.dismissed()
             }
 
             onCancel: {
                 menuModel.activate(menuIndex, false);
+                notification.dismissed()
             }
 
             onEmergencyCall: {
                 shell.startLockedApp("dialer-app");
                 menuModel.activate(menuIndex, false);
+                notification.dismissed()
             }
 
             property var extendedData: menuData && menuData.ext || undefined

=== modified file 'qml/Notifications/Notifications.qml'
--- qml/Notifications/Notifications.qml	2016-05-18 18:14:47 +0000
+++ qml/Notifications/Notifications.qml	2016-05-18 18:14:47 +0000
@@ -41,11 +41,18 @@
         filterRegExp: RegExp(UnityNotifications.Notification.SnapDecision)
     }
 
-    property bool topmostIsFullscreen: false
+    readonly property bool topmostIsFullscreen: fullscreenIndex != -1
     spacing: topmostIsFullscreen ? 0 : units.gu(1)
 
     currentIndex: count > 1 ? 1 : -1
 
+    property int fullscreenIndex: -1
+    onFullscreenIndexChanged: {
+        if (fullscreenIndex != -1) {
+            positionViewAtIndex(fullscreenIndex, ListView.Beginning);
+        }
+    }
+
     delegate: Notification {
         objectName: "notification" + index
         width: parent.width
@@ -69,16 +76,18 @@
         // FIXME: disabled all transitions because of LP: #1354406 workaround
         //layer.enabled: add.running || remove.running || populate.running
 
-        Component.onCompleted: {
-            if (index == 1) {
-                notificationList.topmostIsFullscreen = fullscreen
+        onFullscreenChanged: updateListTopMostIsFullscreen();
+
+        function updateListTopMostIsFullscreen() {
+            if (fullscreen) {
+                fullscreenIndex = index;
             }
         }
 
-        onFullscreenChanged: {
-            // index 1 because 0 is the PlaceHolder...
-            if (index == 1) {
-                notificationList.topmostIsFullscreen = fullscreen
+        onDismissed: {
+            if (fullscreenIndex == index) {
+                fullscreenIndex = -1;
+                notificationList.positionViewAtBeginning();
             }
         }
     }