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

Proposed by Lukáš Tinkl
Status: Merged
Approved by: Michael Zanetti
Approved revision: 2401
Merged at revision: 2432
Proposed branch: lp://staging/~lukas-kde/unity8/fullscreenNotifications-lp1581498
Merge into: lp://staging/unity8
Prerequisite: lp://staging/~lukas-kde/unity8/notificationExpansionLogicFixes
Diff against target: 153 lines (+37/-14)
3 files modified
qml/Notifications/Notification.qml (+17/-6)
qml/Notifications/NotificationMenuItemFactory.qml (+3/-0)
qml/Notifications/Notifications.qml (+17/-8)
To merge this branch: bzr merge lp://staging/~lukas-kde/unity8/fullscreenNotifications-lp1581498
Reviewer Review Type Date Requested Status
Michael Zanetti (community) Approve
Unity8 CI Bot continuous-integration Needs Fixing
Albert Astals Cid (community) Abstain
Review via email: mp+294799@code.staging.launchpad.net

Commit message

Fullscreen notification bug fixes

Description of the change

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

To post a comment you must log in.
2400. By Lukáš Tinkl

perform the "reject" action unconditionally

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

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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)
Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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)
Revision history for this message
Albert Astals Cid (aacid) wrote :

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
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

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

Revision history for this message
Michael Zanetti (mzanetti) wrote :

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

review: Needs Fixing
Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

> 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.

Revision history for this message
Albert Astals Cid (aacid) wrote :

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

review: Abstain
Revision history for this message
Michael Zanetti (mzanetti) wrote :

well, can also reproduce it with wifi ones still:

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

review: Needs Fixing
2401. By Lukáš Tinkl

better fix for handling the fullscreen state

Revision history for this message
Lukáš Tinkl (lukas-kde) wrote :

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

Revision history for this message
Unity8 CI Bot (unity8-ci-bot) wrote :

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)
Revision history for this message
Michael Zanetti (mzanetti) wrote :

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.

Revision history for this message
Michael Zanetti (mzanetti) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches