Merge lp://qastaging/~lukas-kde/unity8/stagedCloseShortcut into lp://qastaging/unity8
- stagedCloseShortcut
- Merge into trunk
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Approved by: | Michael Terry | ||||||||
Approved revision: | 2419 | ||||||||
Merged at revision: | 2554 | ||||||||
Proposed branch: | lp://qastaging/~lukas-kde/unity8/stagedCloseShortcut | ||||||||
Merge into: | lp://qastaging/unity8 | ||||||||
Diff against target: |
178 lines (+63/-12) 5 files modified
qml/Stages/AbstractStage.qml (+9/-0) qml/Stages/DesktopStage.qml (+8/-8) qml/Stages/PhoneStage.qml (+8/-2) qml/Stages/TabletStage.qml (+8/-2) tests/qmltests/tst_Shell.qml (+30/-0) |
||||||||
To merge this branch: | bzr merge lp://qastaging/~lukas-kde/unity8/stagedCloseShortcut | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Unity8 CI Bot | continuous-integration | Approve | |
Michał Sawicz | Needs Fixing | ||
Michael Terry | Approve | ||
Review via email:
|
Commit message
Provide window/surface close shortcuts across all the stages
Description of the change
Provide window/surface close shortcuts across all the stages
Fixed bug lp:1578392
Also, in desktop stage, prevent dash from being closed using this shortcut
* 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
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A
* If you changed the UI, has there been a design review?
N/A
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
Rather than duplicating the shortcut logic, could we bring it up a layer? Into either AbstractStage or Shell?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
Refactored into AbstractStage
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
Looks better now. I tested on phone, worked fine (no animation though... I don't suppose design has ever mentioned what that should look like?)
Could use a test, as I mentioned on IRC and you are already working on.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2413
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
> Looks better now. I tested on phone, worked fine (no animation though... I
> don't suppose design has ever mentioned what that should look like?)
>
> Could use a test, as I mentioned on IRC and you are already working on.
Test added
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2414
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
This looks good but on top of silo 59 (and maybe even without?), the focus does not correctly move to the next app (on phone, where I tested).
That should be checked in the qmltest as well (which might mean fixing the mocks, as you were saying on IRC?).
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
(Focus is set correctly when closing via the launcher Quit button -- can we just make the same call it does?)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2415
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2416
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2417
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
Phone stage fixed, test (still) passing too
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
OK, this does fix it for me on phones.
- Looks like a test failure in PhoneStage:
- Is the comment at the top of onItemRemoved suggesting that we don't do what you just did? If so, maybe we should clarify with someone that there aren't corner cases (I don't know of any off the top of my head where the first in line isn't always focused, but...). And maybe remove the comment too in that case.
- The desktop stage already set focus correctly?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
> OK, this does fix it for me on phones.
>
> - Looks like a test failure in PhoneStage:
Looks like an unrelated test failure to me; CI passes on xenial and also locally:
Totals: 34 passed, 0 failed, 0 skipped, 0 blacklisted
I think it's just a flakiness; doing it differently to prevent that
> - Is the comment at the top of onItemRemoved suggesting that we don't do what
> you just did? If so, maybe we should clarify with someone that there aren't
> corner cases (I don't know of any off the top of my head where the first in
> line isn't always focused, but...). And maybe remove the comment too in that
> case.
I'll clarify the comment
> - The desktop stage already set focus correctly?
Yup, because it uses FocusScopes internally
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2418
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
OK, LGTM then, thanks!
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2418
https:/
Executed test runs:
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michał Sawicz (saviq) wrote : | # |
Text conflict in tests/qmltests/
- 2419. By Lukáš Tinkl
-
merge trunk, resolve conflict
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
Merged trunk, resolved conflict
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2419
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
UNSTABLE: https:/
UNSTABLE: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2419
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
- 2420. By Lukáš Tinkl
-
merge trunk
PASSED: Continuous integration, rev:2412 /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/1311/ /unity8- jenkins. ubuntu. com/job/ build/1739 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= vivid+overlay, testname= qmluitests. sh/843 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= xenial+ overlay, testname= qmluitests. sh/843 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= yakkety, testname= qmluitests. sh/843 /unity8- jenkins. ubuntu. com/job/ build-0- fetch/1765 /unity8- jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= vivid+overlay/ 1712 /unity8- jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= xenial+ overlay/ 1712 /unity8- jenkins. ubuntu. com/job/ build-1- sourcepkg/ release= yakkety/ 1712 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= vivid+overlay/ 1705 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= vivid+overlay/ 1705/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 1705 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 1705/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= yakkety/ 1705 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= yakkety/ 1705/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= vivid+overlay/ 1705 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= vivid+overlay/ 1705/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 1705 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 1705/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= yakkety/ 1705 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= yakkety/ 1705/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= vivid+overlay/ 1705 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= vivid+overlay/ 1705/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 1705 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 1705/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= yakkety/ 1705 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= yakkety/ 1705/artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild: /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/1311/ rebuild
https:/