Merge lp:~lukas-kde/unity8/terminal-shortcut-lp1673500 into lp:unity8
- terminal-shortcut-lp1673500
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Michael Terry | ||||
Approved revision: | 2874 | ||||
Merged at revision: | 2897 | ||||
Proposed branch: | lp:~lukas-kde/unity8/terminal-shortcut-lp1673500 | ||||
Merge into: | lp:unity8 | ||||
Diff against target: |
104 lines (+57/-0) 4 files modified
qml/Components/KeyboardShortcutsOverlay.qml (+13/-0) qml/Stage/Stage.qml (+20/-0) tests/mocks/Unity/Application/ApplicationManager.cpp (+5/-0) tests/qmltests/tst_Shell.qml (+19/-0) |
||||
To merge this branch: | bzr merge lp:~lukas-kde/unity8/terminal-shortcut-lp1673500 | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Unity8 CI Bot | continuous-integration | Approve | |
Michael Terry | Approve | ||
Daniel d'Andrada (community) | Abstain | ||
Review via email:
|
Commit message
Implement Ctrl+Alt+T to launch the terminal app
Description of the change
Implement Ctrl+Alt+T to launch the terminal app
If the app is already running (in background), it's just raised and focused.
![](/+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)
Daniel d'Andrada (dandrader) wrote : | # |
On Unity 7 a new terminal application is always launched when Ctrl+Alt+T is pressed. And that's how I expect it to behave. If we are looking for feature parity unity8 should work like that as well.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel d'Andrada (dandrader) wrote : | # |
ubuntu-terminal-app UI is still pretty touch-oriented still. Any ideas on how to have Ctrl+Alt+T configurable like in unity 7? There I believe it uses /usr/bin/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel d'Andrada (dandrader) wrote : | # |
> On Unity 7 a new terminal application is always launched when Ctrl+Alt+T is
> pressed. And that's how I expect it to behave. If we are looking for feature
> parity unity8 should work like that as well.
But we don't have support for multiple app instances yet, so nevermind.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
I tested this and it didn't launch the app for me.
+ text: i18n.tr("Starts the terminal application.")
I might capitalize the Terminal there?
+ const termAppName = "ubuntu-
Shouldn't we try to launch the snap as a fallback?
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
@Daniel, I *think* unity7 uses a gnome-provided gsettings to make it configurable. Not sure if we want to rely on that key.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
> Shouldn't we try to launch the snap as a fallback?
Or actually, maybe we should try snap first, and fall back to deb.
- 2870. By Lukáš Tinkl
-
capitalize Terminal
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
> I tested this and it didn't launch the app for me.
Oh, anything relevant in the log?
> + text: i18n.tr("Starts the terminal application.")
>
> I might capitalize the Terminal there?
Fixed
> + const termAppName = "ubuntu-
>
> Shouldn't we try to launch the snap as a fallback?
Yeah maybe, any idea how? :)
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2870
https:/
Executed test runs:
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:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
> Yeah maybe, any idea how? :)
Maybe. Does this current MP work for you? What about something like this (untested):
function launchApp(appId) {
if (root.applicati
return true;
}
return root.applicatio
}
function launchTerminal() {
// Try snap first, then deb package
if (!launchApp(
}
}
(That launchApp function is basically a copy of Shell.qml's startApp. Maybe some re-use could happen there.)
- 2871. By Lukáš Tinkl
-
fix review comments; try a snap pkg first
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
> > Yeah maybe, any idea how? :)
>
> Maybe. Does this current MP work for you? What about something like this
> (untested):
Yup, here it's really called "ubuntu-
Fixed the code to try the snap pkg first, as you suggested.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
Just tried "com.ubuntu.
$ apt-cache policy ubuntu-terminal-app
*** 0.7.218+
500 http://
100 /var/lib/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2871
https:/
Executed test runs:
FAILURE: https:/
SUCCESS: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
FAILURE: https:/
SUCCESS: https:/
deb: https:/
Click here to trigger a rebuild:
https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Daniel d'Andrada (dandrader) wrote : | # |
@mterry unity 7 does use the x-terminal-emulator link from update-alternatives
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michał Sawicz (saviq) wrote : | # |
Can't use ↑ unless we resolve it back into an app id.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2871
https:/
Executed test runs:
SUCCESS: https:/
UNSTABLE: 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:/
- 2872. By Lukáš Tinkl
-
merge trunk
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2872
https:/
Executed test runs:
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:/
- 2873. By Lukáš Tinkl
-
try with a list of candidate app names
- 2874. By Lukáš Tinkl
-
explain the candidates order with a comment
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Michael Terry (mterry) wrote : | # |
Tested, looks good to me. Thanks!
It would be nice if we can investigate options for not hardcoding this in the future, but that shouldn't block this improvement. And this MP doesn't paint us into a corner around hardcoding in the future.
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
FAILED: Continuous integration, rev:2873
https:/
Executed test runs:
SUCCESS: https:/
SUCCESS: https:/
FAILURE: 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:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Lukáš Tinkl (lukas-kde) wrote : | # |
The ^^ zesty failure is due to https:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2873
https:/
Executed test runs:
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:/
![](/+icing/build/overlay/assets/skins/sam/images/close.gif)
Unity8 CI Bot (unity8-ci-bot) wrote : | # |
PASSED: Continuous integration, rev:2874
https:/
Executed test runs:
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:/
Preview Diff
1 | === modified file 'qml/Components/KeyboardShortcutsOverlay.qml' |
2 | --- qml/Components/KeyboardShortcutsOverlay.qml 2017-02-06 15:40:57 +0000 |
3 | +++ qml/Components/KeyboardShortcutsOverlay.qml 2017-03-27 15:21:20 +0000 |
4 | @@ -110,6 +110,19 @@ |
5 | Layout.maximumWidth: maxTextSize |
6 | } |
7 | |
8 | + Label { |
9 | + text: i18n.tr("Ctrl + Alt + T") |
10 | + fontSize: "small" |
11 | + font.weight: Font.Medium |
12 | + } |
13 | + Label { |
14 | + text: i18n.tr("Starts the Terminal application.") |
15 | + fontSize: "small" |
16 | + font.weight: Font.Light |
17 | + wrapMode: Text.Wrap |
18 | + Layout.maximumWidth: maxTextSize |
19 | + } |
20 | + |
21 | |
22 | // Launcher section |
23 | Item { Layout.columnSpan: 2; height: units.gu(2) } |
24 | |
25 | === modified file 'qml/Stage/Stage.qml' |
26 | --- qml/Stage/Stage.qml 2017-03-21 10:54:23 +0000 |
27 | +++ qml/Stage/Stage.qml 2017-03-27 15:21:20 +0000 |
28 | @@ -224,10 +224,30 @@ |
29 | active: priv.focusedAppDelegate !== null |
30 | } |
31 | |
32 | + GlobalShortcut { |
33 | + shortcut: Qt.ControlModifier|Qt.AltModifier|Qt.Key_T |
34 | + onTriggered: { |
35 | + // try in this order: snap pkg, new deb name, old deb name |
36 | + var candidates = ["ubuntu-terminal-app_ubuntu-terminal-app", "ubuntu-terminal-app", "com.ubuntu.terminal"]; |
37 | + for (var i = 0; i < candidates.length; i++) { |
38 | + if (priv.startApp(candidates[i])) |
39 | + break; |
40 | + } |
41 | + } |
42 | + } |
43 | + |
44 | QtObject { |
45 | id: priv |
46 | objectName: "DesktopStagePrivate" |
47 | |
48 | + function startApp(appId) { |
49 | + if (root.applicationManager.findApplication(appId)) { |
50 | + return root.applicationManager.requestFocusApplication(appId); |
51 | + } else { |
52 | + return root.applicationManager.startApplication(appId) !== null; |
53 | + } |
54 | + } |
55 | + |
56 | property var focusedAppDelegate: null |
57 | property var foregroundMaximizedAppDelegate: null // for stuff like drop shadow and focusing maximized app by clicking panel |
58 | |
59 | |
60 | === modified file 'tests/mocks/Unity/Application/ApplicationManager.cpp' |
61 | --- tests/mocks/Unity/Application/ApplicationManager.cpp 2017-02-24 15:43:46 +0000 |
62 | +++ tests/mocks/Unity/Application/ApplicationManager.cpp 2017-03-27 15:21:20 +0000 |
63 | @@ -477,6 +477,11 @@ |
64 | m_availableApplications.append(application); |
65 | |
66 | application = new ApplicationInfo(this); |
67 | + application->setAppId("ubuntu-terminal-app"); |
68 | + application->setName("Terminal"); |
69 | + m_availableApplications.append(application); |
70 | + |
71 | + application = new ApplicationInfo(this); |
72 | application->setAppId("primary-oriented-app"); |
73 | application->setName("Primary Oriented"); |
74 | application->setSupportedOrientations(Qt::PrimaryOrientation); |
75 | |
76 | === modified file 'tests/qmltests/tst_Shell.qml' |
77 | --- tests/qmltests/tst_Shell.qml 2017-03-21 10:55:52 +0000 |
78 | +++ tests/qmltests/tst_Shell.qml 2017-03-27 15:21:20 +0000 |
79 | @@ -1460,6 +1460,25 @@ |
80 | confirmLoggedIn(true); |
81 | } |
82 | |
83 | + function test_terminalShortcut() { |
84 | + loadShell("desktop"); |
85 | + shell.usageScenario = "desktop"; |
86 | + waitForRendering(shell); |
87 | + swipeAwayGreeter(); |
88 | + |
89 | + // not running, should start |
90 | + keyClick(Qt.Key_T, Qt.ControlModifier|Qt.AltModifier); |
91 | + tryCompare(ApplicationManager, "focusedApplicationId", "ubuntu-terminal-app"); |
92 | + |
93 | + // start something else |
94 | + ApplicationManager.startApplication("dialer-app"); |
95 | + tryCompare(ApplicationManager, "focusedApplicationId", "dialer-app"); |
96 | + |
97 | + // terminal running in background, should get focused |
98 | + keyClick(Qt.Key_T, Qt.ControlModifier|Qt.AltModifier); |
99 | + tryCompare(ApplicationManager, "focusedApplicationId", "ubuntu-terminal-app"); |
100 | + } |
101 | + |
102 | function test_launcherInverted_data() { |
103 | return [ |
104 | {tag: "phone", formFactor: "phone", usageScenario: "phone", launcherInverted: true}, |
PASSED: Continuous integration, rev:2869 /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/3453/ /unity8- jenkins. ubuntu. com/job/ build/4547 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= xenial+ overlay, testname= qmluitests. sh/2738 /unity8- jenkins. ubuntu. com/job/ test-0- autopkgtest/ label=amd64, release= zesty,testname= qmluitests. sh/2738 /unity8- jenkins. ubuntu. com/job/ build-0- fetch/4575 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4402 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= xenial+ overlay/ 4402/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4402 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=amd64, release= zesty/4402/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4402 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= xenial+ overlay/ 4402/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4402 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=armhf, release= zesty/4402/ artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4402 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= xenial+ overlay/ 4402/artifact/ output/ *zip*/output. zip /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4402 /unity8- jenkins. ubuntu. com/job/ build-2- binpkg/ arch=i386, release= zesty/4402/ artifact/ output/ *zip*/output. zip
https:/
Executed test runs:
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: /unity8- jenkins. ubuntu. com/job/ lp-unity8- ci/3453/ rebuild
https:/