Merge lp:~flscogna/ubuntu-terminal-app/profile-selection into lp:~ubuntu-terminal-dev/ubuntu-terminal-app/reboot

Proposed by Filippo Scognamiglio
Status: Merged
Approved by: Alan Pope 🍺🐧🐱 πŸ¦„
Approved revision: 77
Merged at revision: 80
Proposed branch: lp:~flscogna/ubuntu-terminal-app/profile-selection
Merge into: lp:~ubuntu-terminal-dev/ubuntu-terminal-app/reboot
Diff against target: 434 lines (+207/-59)
8 files modified
po/com.ubuntu.terminal.pot (+13/-8)
src/app/fileio.cpp (+2/-4)
src/app/qml/KeyboardBar.qml (+31/-9)
src/app/qml/KeyboardRows/KeyboardLayout.qml (+2/-2)
src/app/qml/LayoutsPage.qml (+47/-0)
src/app/qml/SettingsPage.qml (+38/-31)
src/app/qml/TerminalSettings.qml (+69/-5)
src/app/qml/ubuntu-terminal-app.qml (+5/-0)
To merge this branch: bzr merge lp:~flscogna/ubuntu-terminal-app/profile-selection
Reviewer Review Type Date Requested Status
Alan Pope 🍺🐧🐱 πŸ¦„ (community) Approve
Ubuntu Phone Apps Jenkins Bot continuous-integration Approve
Review via email: mp+255077@code.launchpad.net

Commit message

Custom profile visibility in settings

Description of the change

Custom profile visibility in settings. It is possible to show only a subset of the installed layouts.

To post a comment you must log in.
Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

I set no profiles active, but still see the keyboard button. Can we hide that when all keyboard layouts are disabled?

http://people.canonical.com/~alan/screenshots/device-2015-04-13-120147.png

Other than that, looks great.

review: Needs Fixing
Revision history for this message
Filippo Scognamiglio (flscogna) wrote :

Alan, we can theoretically hide the keyboard button, but what about the row? If we hide only the button, we would basically have a black useless rectangle at the bottom.
Let's discuss some options:

1) Leave the current situation, with a useless bar and a useless button. (quite bad but predictable)
2) Hide the keyboard bar when no profiles are selected. (might be confusing?)
3) Force at least one profiles to be active (e.g. your ubuntu layout).

What do you think?

Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

I prefer 2).

If someone doesn't want to 'waste' the screen real-estate used by the layout, so switches them all off, they'll be disappointed that they don't gain space IMO.

If we force a profile to be active, we're forcing that 'waste' of space for users who don't need it.

I would default to having the layouts enabled by default, but if the user actively has chosen to disable them _all_ then they should expect the consequence that they're gone from the main UI. They've already discovered how to disable them, re-enabling them shouldn't be a stretch for them.

What do you think?

Revision history for this message
Filippo Scognamiglio (flscogna) wrote :

We already have a setting to completely disable the keyboard bar (in the main page).
The state which might be confusing here is: "Show Keyboard Bar" is turned on, but all profiles are disabled.

I'd personally not mix profiles selection with bar visibility. I'm thinking some day we'll have a keyboard shortcut to show/hide the bar, and we need to preserve the state (read the profiles enabled).

I'm inclined to think the best solution is to always show the additional bar (provided "Show Keyboard Bar" is enabled), but grey out the button when no profile is selected.
If the users wants to recover the screen real-estate (temporarily or permanently) it is just a tick away, without the need to change anything about its favourites profiles.

I perfectly agree with the default choices.

Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

Ok, you make a good argument. Let's go with your plan.

Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

Are you planning to add the grey icon in this merge? If so, when can we expect that?

Revision history for this message
Filippo Scognamiglio (flscogna) wrote :

Sorry, I completely forgot about this merge proposal. I'll work on it in
the next two days.

2015-05-19 11:22 GMT+02:00 Alan Pope ξƒΏ <email address hidden>:

> Are you planning to add the grey icon in this merge? If so, when can we
> expect that?
> --
>
> https://code.launchpad.net/~flscogna/ubuntu-terminal-app/profile-selection/+merge/255077
> You are the owner of lp:~flscogna/ubuntu-terminal-app/profile-selection.
>

74. By Filippo Scognamiglio

Set the button colour to grey when no profiles are selected.

75. By Filippo Scognamiglio

Merge the latest changes from trunk.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Filippo Scognamiglio (flscogna) wrote :

Alan, when you have some time give this a go. The colour might be tweaked a bit, but I think the result is decent.

Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

There seems to be a breakage in some use cases.

Add a few layouts
Go back to the main terminal screen
Select one as the current one
Go back to layout settings, de-select the one set as current

Expected behaviour:- Switch to another layout (next in list for example), or un-set a layout

Actual behaviour can sometimes be:-

* No layout set, choosing a layout has no effect:-
http://people.canonical.com/~alan/screenshots/device-2015-05-27-155914.png
http://people.canonical.com/~alan/screenshots/device-2015-05-27-155920.png

review: Needs Fixing
76. By Filippo Scognamiglio

Fix unexpected behaviour when unselecting the last profile in the list.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
Filippo Scognamiglio (flscogna) wrote :

Ok, there was an issue when the last profile was active and unselected from the settings. This should work fine now.

77. By Filippo Scognamiglio

Import the latest changes from trunk.

Revision history for this message
Ubuntu Phone Apps Jenkins Bot (ubuntu-phone-apps-jenkins-bot) wrote :
review: Approve (continuous-integration)
Revision history for this message
Alan Pope 🍺🐧🐱 πŸ¦„ (popey) wrote :

That fixed it!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'po/com.ubuntu.terminal.pot'
2--- po/com.ubuntu.terminal.pot 2015-03-23 11:12:46 +0000
3+++ po/com.ubuntu.terminal.pot 2015-06-02 19:52:04 +0000
4@@ -8,7 +8,8 @@
5 msgstr ""
6 "Project-Id-Version: \n"
7 "Report-Msgid-Bugs-To: \n"
8-"POT-Creation-Date: 2015-03-21 13:11+0100\n"
9+
10+"POT-Creation-Date: 2015-04-02 14:49+0200\n"
11 "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
12 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
13 "Language-Team: LANGUAGE <LL@li.org>\n"
14@@ -53,23 +54,27 @@
15 msgid "Authentication failed"
16 msgstr ""
17
18-#: ../src/app/qml/KeyboardBar.qml:157
19+#: ../src/app/qml/KeyboardBar.qml:163
20 msgid "Change Keyboard"
21 msgstr ""
22
23+#: ../src/app/qml/LayoutsPage.qml:27 ../src/app/qml/SettingsPage.qml:36
24+msgid "Layouts"
25+msgstr ""
26+
27 #: ../src/app/qml/SettingsPage.qml:27
28 msgid "Settings"
29 msgstr ""
30
31-#: ../src/app/qml/SettingsPage.qml:36
32+#: ../src/app/qml/SettingsPage.qml:42
33+msgid "Show Keyboard Bar"
34+msgstr ""
35+
36+#: ../src/app/qml/SettingsPage.qml:52
37 msgid "Font Size:"
38 msgstr ""
39
40-#: ../src/app/qml/SettingsPage.qml:59
41-msgid "Show Keyboard Bar"
42-msgstr ""
43-
44-#: ../src/app/qml/SettingsPage.qml:73
45+#: ../src/app/qml/SettingsPage.qml:78
46 msgid "Color Scheme"
47 msgstr ""
48
49
50=== modified file 'src/app/fileio.cpp'
51--- src/app/fileio.cpp 2015-02-03 21:43:38 +0000
52+++ src/app/fileio.cpp 2015-06-02 19:52:04 +0000
53@@ -8,8 +8,7 @@
54 if (sourceUrl.isEmpty())
55 return false;
56
57- QUrl url(sourceUrl);
58- QFile file(url.toLocalFile());
59+ QFile file(sourceUrl);
60 if (!file.open(QFile::WriteOnly | QFile::Truncate))
61 return false;
62
63@@ -23,8 +22,7 @@
64 if (sourceUrl.isEmpty())
65 return "";
66
67- QUrl url(sourceUrl);
68- QFile file(url.toLocalFile());
69+ QFile file(sourceUrl);
70 if (!file.open(QFile::ReadOnly))
71 return "";
72
73
74=== modified file 'src/app/qml/KeyboardBar.qml'
75--- src/app/qml/KeyboardBar.qml 2015-02-14 10:33:32 +0000
76+++ src/app/qml/KeyboardBar.qml 2015-06-02 19:52:04 +0000
77@@ -40,13 +40,16 @@
78 }
79 }
80
81- function createLayoutObject(profileUrl) {
82+ function createLayoutObject(profileObject) {
83 var object = layoutComponent.createObject(keyboardContainer);
84- object.loadProfile(fileIO.read(profileUrl));
85+ object.loadProfile(profileObject);
86 return object;
87 }
88
89 function disableLayout(index) {
90+ if (!isIndexLayoutValid(index))
91+ return;
92+
93 var layoutObject = layoutsList.get(index).layout;
94 layoutObject.visible = false;
95 layoutObject.enabled = false;
96@@ -56,6 +59,9 @@
97 }
98
99 function enableLayout(index) {
100+ if (!isIndexLayoutValid(index))
101+ return;
102+
103 var layoutObject = layoutsList.get(index).layout;
104 layoutObject.visible = true;
105 layoutObject.enabled = true;
106@@ -64,10 +70,13 @@
107 layoutObject.simulateCommand.connect(simulateCommand);
108 }
109
110+ function isIndexLayoutValid(index) {
111+ return (index >= 0 && index < layoutsList.count);
112+ }
113+
114 function selectLayout(index) {
115- if (index < 0 || index >= layoutsList.count)
116+ if (!isIndexLayoutValid(index))
117 return;
118-
119 disableLayout(selectedLayoutIndex);
120 enableLayout(index);
121 selectedLayoutIndex = index;
122@@ -97,13 +106,19 @@
123 }
124
125 function loadProfiles() {
126- for (var i = 0; i < keyboardLayouts.length; i++) {
127+ dropProfiles();
128+
129+ for (var i = 0; i < settings.profilesList.count; i++) {
130+ var profile = settings.profilesList.get(i);
131+ if (!profile.profileVisible)
132+ continue;
133+
134 try {
135- console.log("Loading Layout:", Qt.resolvedUrl(keyboardLayouts[i]));
136- var layoutObject = createLayoutObject(Qt.resolvedUrl(keyboardLayouts[i]));
137+ console.log("Loading Layout:", Qt.resolvedUrl(profile.file));
138+ var layoutObject = createLayoutObject(profile.object);
139 layoutsList.append({layout: layoutObject});
140 } catch (e) {
141- console.error("Error in profile " + keyboardLayouts[i]);
142+ console.error("Error in profile " + profile.file);
143 console.error(e);
144 }
145 }
146@@ -129,9 +144,11 @@
147 }
148 }
149
150+ enabled: layoutsList.count != 0
151+
152 Rectangle {
153 anchors.fill: parent
154- color: UbuntuColors.orange
155+ color: parent.enabled ? UbuntuColors.orange : UbuntuColors.warmGrey
156
157 Icon {
158 scale: 0.5
159@@ -187,6 +204,11 @@
160 }
161 }
162
163+ Connections {
164+ target: settings
165+ onProfilesChanged: rootItem.loadProfiles();
166+ }
167+
168 Component.onDestruction: dropProfiles();
169 Component.onCompleted: loadProfiles();
170 }
171
172=== modified file 'src/app/qml/KeyboardRows/KeyboardLayout.qml'
173--- src/app/qml/KeyboardRows/KeyboardLayout.qml 2015-02-14 10:56:28 +0000
174+++ src/app/qml/KeyboardRows/KeyboardLayout.qml 2015-06-02 19:52:04 +0000
175@@ -68,13 +68,13 @@
176 return objectString;
177 }
178
179- function loadProfile(profileString) {
180+ function loadProfile(profileObject) {
181 dropProfile();
182
183 var maxWidth = 0;
184
185 // This function might raise exceptions which are handled in KeyboardBar.qml
186- var profile = profile = Parser.parseJson(profileString);
187+ var profile = profileObject;
188
189 name = profile.name;
190 short_name = profile.short_name;
191
192=== added file 'src/app/qml/LayoutsPage.qml'
193--- src/app/qml/LayoutsPage.qml 1970-01-01 00:00:00 +0000
194+++ src/app/qml/LayoutsPage.qml 2015-06-02 19:52:04 +0000
195@@ -0,0 +1,47 @@
196+/*
197+ * Copyright (C) 2013, 2014 Canonical Ltd
198+ *
199+ * This program is free software: you can redistribute it and/or modify
200+ * it under the terms of the GNU General Public License version 3 as
201+ * published by the Free Software Foundation.
202+ *
203+ * This program is distributed in the hope that it will be useful,
204+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
205+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
206+ * GNU General Public License for more details.
207+ *
208+ * You should have received a copy of the GNU General Public License
209+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
210+ *
211+ * Authored by: Filippo Scognamiglio <flscogna@gmail.com>
212+ */
213+
214+import QtQuick 2.0
215+import Ubuntu.Components 1.1
216+import Ubuntu.Components.ListItems 1.0 as ListItem
217+
218+Page {
219+ id: rootItem
220+ objectName: "layoutsPage"
221+
222+ title: i18n.tr("Layouts")
223+
224+ onVisibleChanged: {
225+ if (visible === false)
226+ settings.profilesChanged();
227+ }
228+
229+ ListView {
230+ anchors.fill: parent
231+ model: settings.profilesList
232+ delegate: ListItem.Standard {
233+ control: Switch {
234+ checked: profileVisible
235+ onCheckedChanged: {
236+ settings.profilesList.setProperty(index, "profileVisible", checked);
237+ }
238+ }
239+ text: name
240+ }
241+ }
242+}
243
244=== modified file 'src/app/qml/SettingsPage.qml'
245--- src/app/qml/SettingsPage.qml 2015-01-17 02:02:04 +0000
246+++ src/app/qml/SettingsPage.qml 2015-06-02 19:52:04 +0000
247@@ -30,50 +30,57 @@
248 id: mainColumn
249
250 spacing: units.gu(1)
251- anchors { margins: units.gu(2); fill: parent }
252-
253- Label {
254- text: i18n.tr("Font Size:")
255- }
256-
257- Slider {
258- id: slFont
259- objectName: "slFont"
260- anchors { left: parent.left; right: parent.right }
261- minimumValue: 8;
262- maximumValue: 32;
263- onValueChanged: {
264- settings.fontSize = value;
265- }
266- Component.onCompleted: {
267- value = settings.fontSize;
268- }
269- }
270-
271- ListItem.ThinDivider { }
272+ anchors.fill: parent
273+
274+ ListItem.Standard {
275+ text: i18n.tr("Layouts")
276+ progression: true
277+ onClicked: pageStack.push(layoutsPage);
278+ }
279+
280+ ListItem.Standard {
281+ text: i18n.tr("Show Keyboard Bar")
282+ control: Switch {
283+ onCheckedChanged: settings.showKeyboardBar = checked;
284+ Component.onCompleted: checked = settings.showKeyboardBar;
285+ }
286+ }
287
288 ListItem.Empty {
289- showDivider: false
290+ height: units.gu(10)
291 Label {
292- anchors { left: parent.left; verticalCenter: parent.verticalCenter }
293- text: i18n.tr("Show Keyboard Bar")
294+ text: i18n.tr("Font Size:")
295+ x: units.gu(2)
296 }
297- Switch {
298- anchors { right: parent.right; verticalCenter: parent.verticalCenter }
299- onCheckedChanged: settings.showKeyboardBar = checked;
300- Component.onCompleted: checked = settings.showKeyboardBar;
301+ Slider {
302+ id: slFont
303+ objectName: "slFont"
304+ anchors {
305+ left: parent.left
306+ right: parent.right
307+ bottom: parent.bottom
308+ margins: units.gu(2)
309+ }
310+ minimumValue: 8;
311+ maximumValue: 32;
312+ onValueChanged: {
313+ settings.fontSize = value;
314+ }
315+ Component.onCompleted: {
316+ value = settings.fontSize;
317+ }
318 }
319 }
320
321- ListItem.ThinDivider { }
322-
323 OptionSelector {
324 id: colorsSchemeSelector
325 objectName: "colorsSchemeSelector"
326 text: i18n.tr("Color Scheme")
327+ width: parent.width - units.gu(4)
328+ x: units.gu(2)
329
330 // TODO Hackish, but works quite well.
331- containerHeight: parent.height - y - units.gu(4)
332+ containerHeight: parent.height - y - units.gu(6)
333
334 // TODO This is a workaround at the moment.
335 // The application should get them from the c++.
336
337=== modified file 'src/app/qml/TerminalSettings.qml'
338--- src/app/qml/TerminalSettings.qml 2015-01-17 02:02:04 +0000
339+++ src/app/qml/TerminalSettings.qml 2015-06-02 19:52:04 +0000
340@@ -1,9 +1,73 @@
341 import QtQuick 2.0
342 import Qt.labs.settings 1.0
343
344-Settings {
345- property int fontSize: 14
346- property string fontStyle: "Ubuntu Mono"
347- property string colorScheme: "Ubuntu"
348- property bool showKeyboardBar: true
349+import "KeyboardRows/jsonParser.js" as Parser
350+
351+Item {
352+ id: rootItem
353+ property alias fontSize: innerSettings.fontSize
354+ property alias fontStyle: innerSettings.fontStyle
355+ property alias colorScheme: innerSettings.colorScheme
356+ property alias showKeyboardBar: innerSettings.showKeyboardBar
357+
358+ property alias jsonVisibleProfiles: innerSettings.jsonVisibleProfiles
359+
360+ property ListModel profilesList: ListModel {}
361+
362+ signal profilesChanged();
363+
364+ onProfilesChanged: saveProfileVisibleStrings();
365+
366+ function saveProfileVisibleStrings() {
367+ var result = {};
368+
369+ for (var i = 0; i < profilesList.count; i++) {
370+ var profileObject = profilesList.get(i);
371+ result[profileObject.file] = profileObject.profileVisible;
372+ }
373+
374+ jsonVisibleProfiles = JSON.stringify(result);
375+ }
376+
377+ Settings {
378+ id: innerSettings
379+ property int fontSize: 14
380+ property string fontStyle: "Ubuntu Mono"
381+ property string colorScheme: "Ubuntu"
382+ property bool showKeyboardBar: true
383+ property string jsonVisibleProfiles: "[]"
384+ }
385+
386+ // Load the keyboard profiles.
387+ Component.onCompleted: {
388+ var visibleProfiles = undefined;
389+ try {
390+ visibleProfiles = JSON.parse(innerSettings.jsonVisibleProfiles);
391+ } catch (e) {}
392+
393+ function isProfileVisible(profilePath) {
394+ return !(visibleProfiles[profilePath] == false);
395+ }
396+
397+ for (var i = 0; i < keyboardLayouts.length; i++) {
398+ var filePath = keyboardLayouts[i];
399+ var isVisible = isProfileVisible(filePath);
400+
401+ try {
402+ var profileObject = Parser.parseJson(fileIO.read(filePath));
403+
404+ profilesList.append(
405+ {
406+ file: filePath,
407+ profileVisible: isVisible,
408+ object: profileObject,
409+ name: profileObject.name
410+ });
411+ } catch (e) {
412+ console.error("Error in profile", filePath);
413+ console.error(e);
414+ }
415+ }
416+ profilesChanged();
417+ }
418 }
419
420=== modified file 'src/app/qml/ubuntu-terminal-app.qml'
421--- src/app/qml/ubuntu-terminal-app.qml 2014-12-05 22:52:26 +0000
422+++ src/app/qml/ubuntu-terminal-app.qml 2015-06-02 19:52:04 +0000
423@@ -106,6 +106,11 @@
424 id: settingsPage
425 visible: false
426 }
427+
428+ LayoutsPage {
429+ id: layoutsPage
430+ visible: false
431+ }
432 }
433
434 Component.onCompleted: {

Subscribers

People subscribed via source and target branches