Unity 8

Merge lp:~macslow/unity8/modal-snap-decisions into lp:unity8

modal-snap-decisions
Merge into trunk

Proposed by Mirco Müller on 2014-03-14

Status:

Merged

Approved by:

Albert Astals Cid on 2014-05-05

Approved revision:

779

Merged at revision:

874

Proposed branch:

lp:~macslow/unity8/modal-snap-decisions

Merge into:

lp:unity8

Diff against target:

164 lines (+113/-1)

4 files modified

debian/control (+1/-1)
qml/Notifications/Notifications.qml (+11/-0)
qml/Shell.qml (+18/-0)
tests/autopilot/unity8/shell/tests/test_notifications.py (+83/-0)

To merge this branch:

bzr merge lp:~macslow/unity8/modal-snap-decisions

High

Fix Released

Link a bug report

Reviewer	Review Type	Date Requested	Status
Albert Astals Cid (community)			Approve on 2014-05-05
Michał Sawicz		2014-03-14	Approve on 2014-05-05
PS Jenkins bot (community)	continuous-integration		Needs Fixing on 2014-04-30
Jouni Helminen (community)		2014-04-03	Approve on 2014-04-03
Review via email: mp+210988@code.launchpad.net

Commit message

Implemented feature-request from Design for modal snap-decision notifications on the phone. See LP #1285712

Description of the change

Implemented feature-request from Design for modal snap-decision notifications on the phone. See LP #1285712

* Are there any related MPs required for this MP to build/function as expected?
Yes, lp:~macslow/unity-notifications/modal-snap-decisions and lp:~macslow/unity-api/version-bump-to-0.1.3 is needed for this to work.

* Did you perform an exploratory manual test run of your code change and any related functionality?
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?
Since this is a Design-feature request, they are certainly aware and expecting this change. Still waiting on Design-feedback on screencast of implementation shown here: https://www.youtube.com/watch?v=sqqmPBa97U0

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-14:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-14:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-17:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-03-20:

8 + Component.onCompleted: {
9 + if (type == Notification.SnapDecision) {
10 + notificationList.snapDecisionCount++;
11 + }
12 + }
13 +
14 + Component.onDestruction: {
15 + if (type == Notification.SnapDecision) {
16 + notificationList.snapDecisionCount--;
17 + }
18 + }
19 +

That will be racy. Use a SortFilterProxyModel and its .count property instead.

review: Needs Fixing

Revision history for this message

Mirco Müller (macslow) wrote on 2014-03-24:

Fixed with r767.

Revision history for this message

Mirco Müller (macslow) wrote on 2014-03-24:

Please note the added requirement for lp:~macslow/unity-notifications/modal-snap-decisions now.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-24:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-03-28:

Text conflict in qml/Notifications/Notifications.qml

review: Needs Fixing

Revision history for this message

Mirco Müller (macslow) wrote on 2014-03-31:

Merged with trunk and fixed resulting merge-conflict.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-31:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-03-31:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-02:

FAILED: Continuous integration, rev:770
http://jenkins.qa.ubuntu.com/job/unity8-ci/2723/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty/4477/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-trusty-touch/4076/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1593
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/1244
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1248
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1248/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/1244
    FAILURE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-trusty/3870/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4583
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-amd64/4583/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4111
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4111/artifact/work/output/*zip*/output.zip
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-runner-mako/6326/console
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/5579

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Jouni Helminen (jounihelminen) wrote on 2014-04-03:

Looks good! approved

review: Approve

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-03:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-04-03:

Too quick, we still need code reviews ;)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-04-10:

Can you collapse this two lines in one?

+ property bool useModal
+
+ useModal: snapDecisionProxyModel.count > 0

Look a bit weird this way

review: Needs Fixing

Revision history for this message

Mirco Müller (macslow) wrote on 2014-04-10:

Done

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-04-10:

I'm having two autopilot tests when running this+dependencies on the phone. Please have a look.

review: Needs Fixing

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-10:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-11:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-04-11:

Code looks good. Tests pass.

* Did you perform an exploratory manual test run of the code change and any related functionality?
Yes

* Did CI run pass? If not, please explain why.
No, otto is a bit broken.

review: Approve

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-04-14:

Please strip tags, you can use the script:

http://people.canonical.com/~msawicz/unity8/strip-u8-tags.sh

Pass lp:~macslow/unity8/modal-snap-decisions as an argument.

Revision history for this message

Mirco Müller (macslow) wrote on 2014-04-14:

The tag-wiping has been kicked off... taking its time :)

Revision history for this message

Mirco Müller (macslow) wrote on 2014-04-14:

All tags cleaned up.

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-04-15:

A few things I noticed here:

- unity-notification-impl should be bumped in Depends here, in Provides in unity-notifications and in the .pc file in unity-api, otherwise we end up with this relying on new api unavailable in old plugin
- I can still interact with apps behind the darkening
- using ./sd-example-simunlock.py I can see the pin screen not align properly at the top and shift a little

review: Needs Fixing

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-04-15:

Actually the last item from above is a bug in trunk: bug #1308011

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-15:

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

review: Needs Fixing (continuous-integration)

lp:~macslow/unity8/modal-snap-decisions updated on 2014-04-15

775. By Mirco Müller on 2014-04-15: Make sure input is also blocked for apps when snap-decisions show up.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-15:

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

review: Needs Fixing (continuous-integration)

lp:~macslow/unity8/modal-snap-decisions updated on 2014-04-16

776. By Mirco Müller on 2014-04-16: Make sure to depend on unity-notifications-impl-3.
777. By Mirco Müller on 2014-04-16: Make sure build depends on unity-notifications-impl-3 (unity-api 0.1.3).

Revision history for this message

Mirco Müller (macslow) wrote on 2014-04-16:

The requested version-bump is done. See...

https://code.launchpad.net/~macslow/unity-api/version-bump-to-0.1.3/+merge/216122
https://code.launchpad.net/~macslow/unity-notifications/modal-snap-decisions/+merge/212483

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-16:

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-04-18:

8 + unity-notifications-impl-3,

Don't add that in build-dependencies. We already have libunity-api-dev (>= 0.1.3)

16 - qtdeclarative5-unity-notifications-plugin | unity-notifications-impl,
17 + qtdeclarative5-unity-notifications-plugin | unity-notifications-impl-3,

Leave that be.

21 - unity-notifications-impl-2,
22 + unity-notifications-impl-3,

Only this needs to happen.

review: Needs Fixing

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-04-18:

(>= 7.80.6) that is, and that's fine - we don't rely on the changes for our *mocks*, for which this would be necessary.

lp:~macslow/unity8/modal-snap-decisions updated on 2014-04-23

778. By Mirco Müller on 2014-04-23: Only bump the run-time dependency regarding the unity-notifications.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-23:

FAILED: Continuous integration, rev:778
http://jenkins.qa.ubuntu.com/job/unity8-ci/2858/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-trusty-touch/378/console
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1722
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/1379
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1383
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1383/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/1379
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/348/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4612
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4612/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6306

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-04-30:

Text conflict in debian/control
1 conflicts encountered.

review: Needs Fixing

lp:~macslow/unity8/modal-snap-decisions updated on 2014-04-30

779. By Mirco Müller on 2014-04-30: Merged with trunk and resolved merge-conflicts.

Revision history for this message

PS Jenkins bot (ps-jenkins) wrote on 2014-04-30:

FAILED: Continuous integration, rev:779
http://jenkins.qa.ubuntu.com/job/unity8-ci/2886/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-trusty-touch/517/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity-phablet-qmluitests-trusty/1750
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-amd64-ci/1407
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1411
        deb: http://jenkins.qa.ubuntu.com/job/unity8-trusty-armhf-ci/1411/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/unity8-trusty-i386-ci/1407
    FAILURE: http://jenkins.qa.ubuntu.com/job/generic-deb-autopilot-runner-mako/457/console
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4787
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-trusty-armhf/4787/artifact/work/output/*zip*/output.zip
    SUCCESS: http://s-jenkins.ubuntu-ci:8080/job/touch-flash-device/6558

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

review: Needs Fixing (continuous-integration)

Revision history for this message

Michał Sawicz (saviq) wrote on 2014-05-05:

Packaging looks good.

review: Approve

Revision history for this message

Albert Astals Cid (aacid) wrote on 2014-05-05:

Talking with Saviq over IRC he agrees the issues he raised are now fixed.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk

Subscribers

People subscribed via source and target branches

to all changes:

Mirco Müller

Mohammad Rakibul Hasan

Unity Team

 === modified file 'debian/control'
 --- debian/control	2014-04-29 15:08:05 +0000
 +++ debian/control	2014-04-30 08:42:30 +0000
@@ -102,7 +102,7 @@
           qtdeclarative5-unity-notifications-plugin | unity-notifications-impl,
           ubuntu-thumbnailer-impl-0,
           unity-application-impl-2,
--         unity-notifications-impl-2,
++         unity-notifications-impl-3,
           unity-plugin-scopes | unity-scopes-impl,
           unity-scopes-impl-0,
           unity8-fake-env | unity-application-impl,
 === modified file 'qml/Notifications/Notifications.qml'
 --- qml/Notifications/Notifications.qml	2014-03-17 15:31:06 +0000
 +++ qml/Notifications/Notifications.qml	2014-04-30 08:42:30 +0000
@@ -16,6 +16,8 @@
  import QtQuick 2.0
  import Ubuntu.Components 0.1
++import Unity.Notifications 1.0 as UnityNotifications
++import Utils 0.1
  import "../Components"
  ListView {
@@ -25,6 +27,15 @@
      interactive: false
      property real margin
++    property bool useModal: snapDecisionProxyModel.count > 0
++
++    SortFilterProxyModel {
++        id: snapDecisionProxyModel
++
++        model: notificationList.model
++        filterRole: UnityNotifications.ModelInterface.RoleType
++        filterRegExp: RegExp(UnityNotifications.Notification.SnapDecision)
++    }
      spacing: delegate.fullscreen ? 0 : units.gu(.5)
      currentIndex: (currentIndex < 1 && count > 1) ? 1 : -1
 === modified file 'qml/Shell.qml'
 --- qml/Shell.qml	2014-04-17 15:43:16 +0000
 +++ qml/Shell.qml	2014-04-30 08:42:30 +0000
@@ -582,6 +582,24 @@
+             }
+         }
++        Rectangle {
++            id: modalNotificationBackground
++
++            visible: notifications.useModal && !greeter.shown && (notifications.state == "narrow")
++            color: "#000000"
++            anchors.fill: parent
++            opacity: 0.5
++
++            MouseArea {
++                anchors.fill: parent
++            }
++
++            InputFilterArea {
++                anchors.fill: parent
++                blockInput: modalNotificationBackground.visible
++            }
++        }
++
          Notifications {
              id: notifications
 === modified file 'tests/autopilot/unity8/shell/tests/test_notifications.py'
 --- tests/autopilot/unity8/shell/tests/test_notifications.py	2014-04-29 15:21:20 +0000
 +++ tests/autopilot/unity8/shell/tests/test_notifications.py	2014-04-30 08:42:30 +0000
@@ -211,6 +211,89 @@
          self.touch.tap_object(notification.select_single(objectName="button4"))
          self.assert_notification_action_id_was_called("action_decline_4")
++    def test_modal_sd_without_greeter (self):
++        """A snap-decision on a phone should block input to shell beneath it when there's no greeter."""
++        unity_proxy = self.launch_unity()
++        unlock_unity(unity_proxy)
++
++        summary = "Incoming file"
++        body = "Frank would like to send you the file: essay.pdf"
++        icon_path = "sync-idle"
++        hints = [
++            ("x-canonical-snap-decisions", "true"),
++            ("x-canonical-non-shaped-icon", "true"),
++        ]
++
++        actions = [
++            ('action_accept', 'Accept'),
++            ('action_decline_1', 'Decline'),
++        ]
++
++        self._create_interactive_notification(
++            summary,
++            body,
++            icon_path,
++            "NORMAL",
++            actions,
++            hints
++        )
++
++        # verify that we cannot reveal the launcher (no longer interact with the shell)
++        time.sleep(1)
++        self.main_window.show_dash_swiping()
++        launcher = self.main_window.get_launcher()
++        self.assertThat(launcher.shown, Eventually(Equals(False)))
++
++        # verify and interact with the triggered snap-decision notification
++        notify_list = self._get_notifications_list()
++        get_notification = lambda: notify_list.wait_select_single(
++            'Notification', objectName='notification1')
++        notification = get_notification()
++        self._assert_notification(notification, summary, body, True, False, 1.0)
++        self.touch.tap_object(notification.select_single(objectName="button0"))
++        self.assert_notification_action_id_was_called("action_accept")
++
++    def test_modal_sd_with_greeter (self):
++        """A snap-decision on a phone should not block input to the greeter beneath it."""
++        unity_proxy = self.launch_unity()
++
++        summary = "Incoming file"
++        body = "Frank would like to send you the file: essay.pdf"
++        icon_path = "sync-idle"
++        hints = [
++            ("x-canonical-snap-decisions", "true"),
++            ("x-canonical-non-shaped-icon", "true"),
++        ]
++
++        actions = [
++            ('action_accept', 'Accept'),
++            ('action_decline_1', 'Decline'),
++        ]
++
++        self._create_interactive_notification(
++            summary,
++            body,
++            icon_path,
++            "NORMAL",
++            actions,
++            hints
++        )
++
++        # verify that we can swipe away the greeter (interact with the "shell")
++        time.sleep(1)
++        self.main_window.show_dash_swiping()
++        greeter = self.main_window.get_greeter()
++        self.assertThat(greeter.shown, Eventually(Equals(False)))
++
++        # verify and interact with the triggered snap-decision notification
++        notify_list = self._get_notifications_list()
++        get_notification = lambda: notify_list.wait_select_single(
++            'Notification', objectName='notification1')
++        notification = get_notification()
++        self._assert_notification(notification, summary, body, True, False, 1.0)
++        self.touch.tap_object(notification.select_single(objectName="button0"))
++        self.assert_notification_action_id_was_called("action_accept")
++
      def _create_interactive_notification(
          self,
          summary="",