Merge lp:~unity-team/unity8/keymapSwitching into lp:unity8
| Status: | Merged | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Approved by: | Michael Terry on 2016-03-15 | ||||||||||||
| Approved revision: | 2281 | ||||||||||||
| Merged at revision: | 2291 | ||||||||||||
| Proposed branch: | lp:~unity-team/unity8/keymapSwitching | ||||||||||||
| Merge into: | lp:unity8 | ||||||||||||
| Prerequisite: | lp:~unity-team/unity8/shell_chrome | ||||||||||||
| Diff against target: |
634 lines (+265/-10) 20 files modified
plugins/AccountsService/AccountsService.cpp (+26/-0) plugins/AccountsService/AccountsService.h (+6/-0) qml/Shell.qml (+50/-0) qml/Stages/AbstractStage.qml (+1/-0) qml/Stages/ApplicationWindow.qml (+4/-0) qml/Stages/DesktopStage.qml (+4/-0) qml/Stages/PhoneStage.qml (+2/-0) qml/Stages/SpreadDelegate.qml (+1/-0) qml/Stages/SurfaceContainer.qml (+13/-0) qml/Stages/TabletStage.qml (+2/-0) tests/mocks/AccountsService/AccountsService.cpp (+17/-0) tests/mocks/AccountsService/AccountsService.h (+8/-0) tests/mocks/QMenuModel/QDBusActionGroup.qml (+5/-5) tests/mocks/Unity/Application/MirSurface.cpp (+18/-2) tests/mocks/Unity/Application/MirSurface.h (+5/-0) tests/plugins/AccountsService/PropertiesServer.cpp (+9/-0) tests/plugins/AccountsService/PropertiesServer.h (+0/-1) tests/plugins/AccountsService/client.cpp (+30/-0) tests/qmltests/Stages/tst_DesktopStage.qml (+0/-2) tests/qmltests/tst_Shell.qml (+64/-0) |
||||||||||||
| To merge this branch: | bzr merge lp:~unity-team/unity8/keymapSwitching | ||||||||||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Daniel d'Andrada (community) | Needs Information on 2016-03-16 | ||
| Michael Terry | 2016-03-11 | Approve on 2016-03-15 | |
| Unity8 CI Bot | continuous-integration | 2016-03-11 | Approve on 2016-03-14 |
|
Review via email:
|
|||
This proposal supersedes a proposal from 2016-03-11.
Commit Message
Keymap switching support
Description of the Change
* Are there any related MPs required for this MP to build/function as expected? Please list.
lp:~unity-team/unity-api/kbdLayout
lp:~unity-team/qtmir/kbdLayout
lp:~lukas-kde/qtubuntu/kbdLayout
* Did you perform an exploratory manual test run of your code change and any related functionality?
Will in silo
* Did you make sure that your branch does not contain spurious tags?
Y
* If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
Y
* If you changed the UI, has there been a design review?
N/A
| Michael Terry (mterry) wrote : | # |
Some inline comments. Haven't reviewed test code yet. Am still setting up my phone to test these branches there.
| Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2276
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
| Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2277
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
| Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2278
https:/
Executed test runs:
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:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
- 2279. By Lukáš Tinkl on 2016-03-12
-
refine test for switching keymaps, move to tst_Shell
| Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2279
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
| Michael Terry (mterry) wrote : | # |
As mentioned in telegram, I think we can drop activeKeymap. And the qmltest should test going past the front / end of keymap list. But besides that, it looks fine. Haven't tested yet, waiting for silo to rebuild.
- 2280. By Lukáš Tinkl on 2016-03-14
-
remove activeKeymapIndex, refine tests
- 2281. By Lukáš Tinkl on 2016-03-14
-
remove debug
| Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2280
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/
| Lukáš Tinkl (lukas-kde) wrote : | # |
> As mentioned in telegram, I think we can drop activeKeymap. And the qmltest
> should test going past the front / end of keymap list. But besides that, it
> looks fine. Haven't tested yet, waiting for silo to rebuild.
Fixed both issues
| Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2281
https:/
Executed test runs:
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:/
Click here to trigger a rebuild:
https:/
| Michael Terry (mterry) wrote : | # |
Note to self:
sudo gdbus call --system --dest org.freedesktop
| Michael Terry (mterry) wrote : | # |
OK, worked fine in my testing. Modulo a crasher if you give invalid input. (bug 1557634) But that's not a blocker for now.
Thanks!
| Daniel d'Andrada (dandrader) wrote : | # |
"""
function switchToKeymap(
var finalKeymap = keymap.split("+");
savedKeymap = keymap; // save the keymap in case the surface changes later
if (surface) {
}
}
"""
I know I'm late to the party, but as I'm rebasing surface-based WM on top of silo 041 I couldn't help frowning when I saw that.
This kind of imperative work doesn't belong to QML. Can't the surface have a property that takes the full string and does any and all parsing needed behind the scenes?
Something like that:
"""
SurfaceContainer {
id: root
property string keymap
Binding {
target: surface
property: "keymap"
value: root.keymap
}
}
"""
| Daniel d'Andrada (dandrader) wrote : | # |
And as a continuation of the idea in my previous comment, in ApplicationWindow:
"""
function switchToKeymap(
}
"""
Would be something like:
"""
property alias keymap: sessionContaine
"""
| Lukáš Tinkl (lukas-kde) wrote : | # |
Yeah, thanks for the comments, I'm totally aware this will need some rework with surface based WM (this and the switching based on the currently focused _app_, not surface).
| Daniel d'Andrada (dandrader) wrote : | # |
My comments are not related to surface-based versus application-based WM. They are about imperative versus declarative code.
FAILED: Continuous integration, rev:2275
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
SUCCESS: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
FAILURE: https:/
Click here to trigger a rebuild:
https:/