Merge lp:~lukas-kde/unity8/cursorHiding into lp:unity8
| Status: | Superseded |
|---|---|
| Proposed branch: | lp:~lukas-kde/unity8/cursorHiding |
| Merge into: | lp:unity8 |
| Prerequisite: | lp:~dandrader/unity8/cursorThemes |
| Diff against target: |
2228 lines (+1412/-128) 47 files modified
debian/unity8-private.install (+1/-0) plugins/CMakeLists.txt (+1/-0) plugins/Cursor/MousePointer.cpp (+4/-0) plugins/Cursor/MousePointer.h (+1/-0) plugins/UInput/CMakeLists.txt (+7/-0) plugins/UInput/UInput.qmltypes (+44/-0) plugins/UInput/plugin.cpp (+26/-0) plugins/UInput/plugin.h (+32/-0) plugins/UInput/qmldir (+3/-0) plugins/UInput/uinput.cpp (+182/-0) plugins/UInput/uinput.h (+61/-0) plugins/Utils/CMakeLists.txt (+1/-1) plugins/Utils/WindowInputMonitor.cpp (+19/-12) plugins/Utils/WindowInputMonitor.h (+37/-20) plugins/Utils/plugin.cpp (+2/-2) qml/Components/InputMethod.qml (+1/-1) qml/Components/VirtualTouchPad.qml (+138/-0) qml/DisabledScreenNotice.qml (+73/-20) qml/OrientedShell.qml (+28/-14) qml/Shell.qml (+15/-6) qml/Stages/AbstractStage.qml (+5/-1) qml/Stages/DesktopStage.qml (+6/-1) qml/Stages/PhoneStage.qml (+5/-1) qml/Stages/TabletStage.qml (+5/-1) tests/mocks/CMakeLists.txt (+1/-0) tests/mocks/Cursor/Cursor.qml (+1/-0) tests/mocks/UInput/CMakeLists.txt (+7/-0) tests/mocks/UInput/mockuinput.cpp (+73/-0) tests/mocks/UInput/mockuinput.h (+63/-0) tests/mocks/UInput/plugin.cpp (+26/-0) tests/mocks/UInput/plugin.h (+32/-0) tests/mocks/UInput/qmldir (+2/-0) tests/mocks/Unity/CMakeLists.txt (+1/-0) tests/mocks/Unity/Screens/CMakeLists.txt (+14/-0) tests/mocks/Unity/Screens/plugin.cpp (+29/-0) tests/mocks/Unity/Screens/plugin.h (+25/-0) tests/mocks/Unity/Screens/qmldir (+2/-0) tests/mocks/Unity/Screens/screens.cpp (+71/-0) tests/mocks/Unity/Screens/screens.h (+82/-0) tests/mocks/Utils/WindowInputMonitor.qml (+3/-1) tests/mocks/Utils/qmldir (+1/-1) tests/plugins/Utils/CMakeLists.txt (+1/-1) tests/plugins/Utils/WindowInputMonitorTest.cpp (+14/-14) tests/qmltests/CMakeLists.txt (+1/-0) tests/qmltests/Components/tst_VirtualTouchPad.qml (+143/-0) tests/qmltests/tst_DisabledScreenNotice.qml (+10/-2) tests/qmltests/tst_OrientedShell.qml (+113/-29) |
| To merge this branch: | bzr merge lp:~lukas-kde/unity8/cursorHiding |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Daniel d'Andrada (community) | 2015-12-01 | Approve on 2015-12-03 | |
| PS Jenkins bot | continuous-integration | Needs Fixing on 2015-12-03 | |
|
Review via email:
|
|||
This proposal has been superseded by a proposal from 2016-01-28.
Commit Message
Hide/reveal the mouse pointer on touch/mouse events
Description of the Change
Hide the mouse pointer when a touch event is detected, reveal it again when the mouse is moved again.
This matches unity7 behavior in this regard, the mouse pointer is moved to the last known touch position.
* Are there any related MPs required for this MP to build/function as expected? Please list.
Yes, lp:~dandrader/unity8/cursorThemes
* 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?
N/A, matches unity7 behavior
| Daniel d'Andrada (dandrader) wrote : | # |
I wouldn't move the cursor because of touch interaction.
That's an habit of X11, because of its "low-level" mouse emulation out of touch events.
| Daniel d'Andrada (dandrader) wrote : | # |
If you plan to use HomeKeyWatcher for that stuff it should be renamed to something more generic, like WindowInputMonitor or something.
| Lukáš Tinkl (lukas-kde) wrote : | # |
> I wouldn't move the cursor because of touch interaction.
>
> That's an habit of X11, because of its "low-level" mouse emulation out of
> touch events.
Discussed this with mzanetti and I also compared it with other systems (unity7, Windows 10), it really feels natural this way. Of course, subjective feeling, we can ask design for opinion on this
| Lukáš Tinkl (lukas-kde) wrote : | # |
> If you plan to use HomeKeyWatcher for that stuff it should be renamed to
> something more generic, like WindowInputMonitor or something.
Yup, will do
- 2079. By Lukáš Tinkl on 2015-12-02
-
rename HomeKeyWatcher.cpp => WindowInputMoni
tor.cpp
| Lukáš Tinkl (lukas-kde) wrote : | # |
> If you plan to use HomeKeyWatcher for that stuff it should be renamed to
> something more generic, like WindowInputMonitor or something.
Done
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2079
http://
Executed test runs:
UNSTABLE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Daniel d'Andrada (dandrader) wrote : | # |
You also have to update/rephrase WindowInputMoni
I would suggest some changes to those new signal names to get them closer to existing naming and concepts:
s/windowTouched
s/windowRelease
Also the class has "Window" in its name already, so no need to repeat that in the signal name.
------------
In WindowInputMoni
"""
if (touchEv && !touchEv-
"""
A touch event is malformed if it doesn't contain any touch point. So that should be an assertion if you care about checking it at all.
Furthermore, it would be inconsistent to emit a touchBegun() but not its touchEnded() counterpart.
| Daniel d'Andrada (dandrader) wrote : | # |
In Shell.qml
"""
// store the last known touch position
"""
Not a very accurate comment. You're not "storing" the touch position, you're moving the Cursor to it.
| Daniel d'Andrada (dandrader) wrote : | # |
In Shell.qml
"""
function revealCursor() {
if (touchDetected) {
}
}
"""
There should be no need to touch Mir.cursorName.
| Daniel d'Andrada (dandrader) wrote : | # |
Can't you move the stuff in cursorPriv to Cursor.qml?
- 2080. By Lukáš Tinkl on 2015-12-02
-
rename signals, add some docu
- 2081. By Lukáš Tinkl on 2015-12-02
-
move the hide/reveal functions to Cursor.qml
| Lukáš Tinkl (lukas-kde) wrote : | # |
Comments/issues addressed
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2081
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Daniel d'Andrada (dandrader) wrote : | # |
I think there's quite a bit of redundant code there. I removed it and it works fine:
http://
In Shell.qml:
"""
// move the (hidden) cursor to the last known touch position
}
"""
You can't simply assign as the touch position is in a different coordinate system. Cursor is in Shell coordinates whereas touch position is in root, OrientedShell, coordinates. You have to map it.
You can see it's broken is you try it on a Nexus 7 for instance.
- 2082. By Lukáš Tinkl on 2015-12-03
-
simplify code
- 2083. By Lukáš Tinkl on 2015-12-03
-
map the touch coords
- 2084. By Lukáš Tinkl on 2015-12-03
-
unbreak tests after the class renaming and new signal additions
| Lukáš Tinkl (lukas-kde) wrote : | # |
Comments addressed, also fixed the tests after the class renaming and new signal additions
- 2085. By Lukáš Tinkl on 2015-12-03
-
simplify, the item mapped from can be null
| Daniel d'Andrada (dandrader) wrote : | # |
In Shell.qml:
"""
var mappedCoords = mapFromItem(
"""
When mapping from the root coordinate system you just pass null, like this:
var mappedCoords = mapFromItem(null, pos.x, pos.y);
- 2086. By Lukáš Tinkl on 2015-12-03
-
map from null
- 2087. By Lukáš Tinkl on 2015-12-03
-
improve docu
- 2088. By Lukáš Tinkl on 2015-12-03
-
revert unrelated code changes
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2082
http://
Executed test runs:
UNSTABLE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Daniel d'Andrada (dandrader) wrote : | # |
Works fine and code looks ok. Let's wait for jenkins before top-approving.
| PS Jenkins bot (ps-jenkins) wrote : | # |
FAILED: Continuous integration, rev:2088
http://
Executed test runs:
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild:
http://
| Daniel d'Andrada (dandrader) wrote : | # |
* 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.
Yes (apart from the usual xenial AP failure)
* Did you make sure that the branch does not contain spurious tags?
Yes
- 2089. By Lukáš Tinkl on 2015-12-16
-
remove conflicting empty line
- 2090. By Lukáš Tinkl on 2015-12-16
-
+s
- 2091. By Lukáš Tinkl on 2016-01-15
-
merge trunk
- 2092. By Lukáš Tinkl on 2016-01-28

FAILED: Continuous integration, rev:2078 jenkins. qa.ubuntu. com/job/ unity8- ci/6860/ jenkins. qa.ubuntu. com/job/ generic- deb-autopilot- vivid-touch/ 5538/console jenkins. qa.ubuntu. com/job/ generic- deb-autopilot- xenial- touch/275/ console jenkins. qa.ubuntu. com/job/ unity-phablet- qmluitests- vivid/1571 jenkins. qa.ubuntu. com/job/ unity8- qmluitest- xenial- amd64/274 jenkins. qa.ubuntu. com/job/ unity8- vivid-amd64- ci/1466 jenkins. qa.ubuntu. com/job/ unity8- vivid-i386- ci/1466 jenkins. qa.ubuntu. com/job/ unity8- xenial- amd64-ci/ 273 jenkins. qa.ubuntu. com/job/ unity8- xenial- i386-ci/ 272 jenkins. qa.ubuntu. com/job/ generic- deb-autopilot- runner- vivid-touch/ 4335/console jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- vivid-armhf/ 5552 jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- vivid-armhf/ 5552/artifact/ work/output/ *zip*/output. zip s-jenkins. ubuntu- ci:8080/ job/touch- flash-device/ 25684 jenkins. qa.ubuntu. com/job/ generic- deb-autopilot- runner- xenial- touch/95/ console jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- xenial- armhf/274 jenkins. qa.ubuntu. com/job/ generic- mediumtests- builder- xenial- armhf/274/ artifact/ work/output/ *zip*/output. zip s-jenkins. ubuntu- ci:8080/ job/touch- flash-device/ 25683
http://
Executed test runs:
FAILURE: http://
FAILURE: http://
UNSTABLE: http://
UNSTABLE: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
FAILURE: http://
SUCCESS: http://
deb: http://
SUCCESS: http://
Click here to trigger a rebuild: s-jenkins. ubuntu- ci:8080/ job/unity8- ci/6860/ rebuild
http://