Merge lp:~zsombi/ubuntu-ui-toolkit/mnemonics into lp:ubuntu-ui-toolkit/staging

Proposed by Zsombor Egri on 2015-12-06
Status: Merged
Approved by: Christian Dywan on 2015-12-18
Approved revision: 1748
Merged at revision: 1785
Proposed branch: lp:~zsombi/ubuntu-ui-toolkit/mnemonics
Merge into: lp:ubuntu-ui-toolkit/staging
Prerequisite: lp:~zsombi/ubuntu-ui-toolkit/reset_action_shortcut
Diff against target: 404 lines (+203/-20)
7 files modified
examples/ubuntu-ui-toolkit-gallery/Buttons.qml (+2/-2)
examples/ubuntu-ui-toolkit-gallery/MainPage.qml (+12/-0)
src/Ubuntu/Components/plugin/quickutils.cpp (+2/-1)
src/Ubuntu/Components/plugin/quickutils.h (+7/-0)
src/Ubuntu/Components/plugin/ucaction.cpp (+114/-16)
src/Ubuntu/Components/plugin/ucaction.h (+8/-1)
tests/unit_x11/tst_components/tst_shortcuts.qml (+58/-0)
To merge this branch: bzr merge lp:~zsombi/ubuntu-ui-toolkit/mnemonics
Reviewer Review Type Date Requested Status
PS Jenkins bot continuous-integration Approve on 2015-12-18
Christian Dywan (community) 2015-12-06 Approve on 2015-12-18
Review via email: mp+279687@code.launchpad.net

Commit message

Add mnemonics support to Action.

To post a comment you must log in.
1744. By Zsombor Egri on 2015-12-10

prereq sync

1745. By Zsombor Egri on 2015-12-10

documentation updates, build fixes, test adjustments

1746. By Zsombor Egri on 2015-12-10

staging sync

Christian Dywan (kalikiana) wrote :

- m_mouseAttached(false)
47 + m_mouseAttached(true),

The default must not change.

// FIXME we should only do this if we have HW keyboard attached
+ if (QuickUtils::instance().keyboardAttached()) {

The comment doesn't make sense - you have an API right there. Better make it a FIXME with a reference to lp#.

+ // FIXME: do this with a proper API
+ connect(&QuickUtils::instance(), &QuickUtils::keyboardAttachedChanged,
+ this, &UCAction::onKeyboardAttached);

Again, add a FIXME with a reference to lp# matching the above.

        function test_mnemonic_data() {

How about more corner cases that could potentially crash? Such as "Hit& Space", "Paste &&&Proceed", "Jump &".

review: Needs Fixing
Zsombor Egri (zsombi) wrote :

> - m_mouseAttached(false)
> 47 + m_mouseAttached(true),
>
> The default must not change.

Ahh, that's a change which was introduced by the landing fix, where the default got set to false. Good catch!!

>
> // FIXME we should only do this if we have HW keyboard attached
> + if (QuickUtils::instance().keyboardAttached()) {
>
> The comment doesn't make sense - you have an API right there. Better make it a
> FIXME with a reference to lp#.

right... done.

>
> + // FIXME: do this with a proper API
> + connect(&QuickUtils::instance(), &QuickUtils::keyboardAttachedChanged,
> + this, &UCAction::onKeyboardAttached);
>
> Again, add a FIXME with a reference to lp# matching the above.

Same here.

>
> function test_mnemonic_data() {
>
> How about more corner cases that could potentially crash? Such as "Hit&
> Space", "Paste &&&Proceed", "Jump &".

Added them, and I have to check more...

1747. By Zsombor Egri on 2015-12-18

staging sync

1748. By Zsombor Egri on 2015-12-18

more tests added, revealing that we needed more precise positioning of the underlined mnemonic

Christian Dywan (kalikiana) wrote :

Nice work on the tests!

And I like very much that you're now using the sequence to find the mnemonic for display purposes. That way we'll never see an underline for an unsupported key.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'examples/ubuntu-ui-toolkit-gallery/Buttons.qml'
2--- examples/ubuntu-ui-toolkit-gallery/Buttons.qml 2015-06-19 05:14:00 +0000
3+++ examples/ubuntu-ui-toolkit-gallery/Buttons.qml 2015-12-18 15:21:18 +0000
4@@ -50,8 +50,8 @@
5 objectName: "button_color"
6 width: units.gu(20)
7 action: Action {
8- text: i18n.tr("Call %1").arg(shortcut)
9- shortcut: 'Ctrl+C'
10+ text: i18n.tr("C&all %1").arg(shortcut)
11+ shortcut: 'Ctrl+L'
12 property bool flipped
13 onTriggered: flipped = !flipped
14 }
15
16=== modified file 'examples/ubuntu-ui-toolkit-gallery/MainPage.qml'
17--- examples/ubuntu-ui-toolkit-gallery/MainPage.qml 2015-12-16 11:53:01 +0000
18+++ examples/ubuntu-ui-toolkit-gallery/MainPage.qml 2015-12-18 15:21:18 +0000
19@@ -66,6 +66,18 @@
20 iconName: "starred"
21 visible: !QuickUtils.mouseAttached
22 onTriggered: QuickUtils.mouseAttached = true
23+ },
24+ Action {
25+ text: i18n.tr("Detach keyboard")
26+ iconName: "non-starred"
27+ visible: QuickUtils.keyboardAttached
28+ onTriggered: QuickUtils.keyboardAttached = false
29+ },
30+ Action {
31+ text: i18n.tr("Attach keyboard")
32+ iconName: "starred"
33+ visible: !QuickUtils.keyboardAttached
34+ onTriggered: QuickUtils.keyboardAttached = true
35 }
36 ]
37 }
38
39=== modified file 'src/Ubuntu/Components/plugin/quickutils.cpp'
40--- src/Ubuntu/Components/plugin/quickutils.cpp 2015-12-07 09:07:00 +0000
41+++ src/Ubuntu/Components/plugin/quickutils.cpp 2015-12-18 15:21:18 +0000
42@@ -33,7 +33,8 @@
43 QuickUtils::QuickUtils(QObject *parent) :
44 QObject(parent),
45 m_rootView(0),
46- m_mouseAttached(false)
47+ m_mouseAttached(false),
48+ m_keyboardAttached(false)
49 {
50 QGuiApplication::instance()->installEventFilter(this);
51 m_omitIM << "ibus" << "none" << "compose";
52
53=== modified file 'src/Ubuntu/Components/plugin/quickutils.h'
54--- src/Ubuntu/Components/plugin/quickutils.h 2015-11-05 13:41:35 +0000
55+++ src/Ubuntu/Components/plugin/quickutils.h 2015-12-18 15:21:18 +0000
56@@ -32,6 +32,7 @@
57 Q_PROPERTY(QString inputMethodProvider READ inputMethodProvider)
58 Q_PROPERTY(bool touchScreenAvailable READ touchScreenAvailable NOTIFY touchScreenAvailableChanged)
59 Q_PROPERTY(bool mouseAttached MEMBER m_mouseAttached NOTIFY mouseAttachedChanged)
60+ Q_PROPERTY(bool keyboardAttached MEMBER m_keyboardAttached NOTIFY keyboardAttachedChanged)
61 public:
62 static QuickUtils& instance()
63 {
64@@ -53,6 +54,10 @@
65 {
66 return m_mouseAttached;
67 }
68+ bool keyboardAttached()
69+ {
70+ return m_keyboardAttached;
71+ }
72
73 Q_SIGNALS:
74 void rootObjectChanged();
75@@ -60,6 +65,7 @@
76 void deactivated();
77 void touchScreenAvailableChanged();
78 void mouseAttachedChanged();
79+ void keyboardAttachedChanged();
80
81 protected:
82 bool eventFilter(QObject *, QEvent *);
83@@ -69,6 +75,7 @@
84 QPointer<QQuickView> m_rootView;
85 QStringList m_omitIM;
86 bool m_mouseAttached;
87+ bool m_keyboardAttached;
88
89 void lookupQuickView();
90 };
91
92=== modified file 'src/Ubuntu/Components/plugin/ucaction.cpp'
93--- src/Ubuntu/Components/plugin/ucaction.cpp 2015-12-13 07:48:56 +0000
94+++ src/Ubuntu/Components/plugin/ucaction.cpp 2015-12-18 15:21:18 +0000
95@@ -15,6 +15,7 @@
96 */
97
98 #include "ucaction.h"
99+#include "quickutils.h"
100
101 #include <QtDebug>
102 #include <QtQml/QQmlInfo>
103@@ -22,6 +23,30 @@
104 #include <QtQuick/qquickwindow.h>
105 #include <private/qguiapplication_p.h>
106
107+bool shortcutContextMatcher(QObject* object, Qt::ShortcutContext context)
108+{
109+ UCAction* action = static_cast<UCAction*>(object);
110+ // Can't access member here because it's not public
111+ if (!action->property("enabled").toBool())
112+ return false;
113+
114+ switch (context) {
115+ case Qt::ApplicationShortcut:
116+ return true;
117+ case Qt::WindowShortcut: {
118+ QObject* window = object;
119+ while (window && !window->isWindowType()) {
120+ window = window->parent();
121+ if (QQuickItem* item = qobject_cast<QQuickItem*>(window))
122+ window = item->window();
123+ }
124+ return window && window == QGuiApplication::focusWindow();
125+ }
126+ default: break;
127+ }
128+ return false;
129+}
130+
131 /*!
132 * \qmltype Action
133 * \instantiates UCAction
134@@ -74,6 +99,20 @@
135 * as well as to define actions for pages, or when defining options in \c ListItemOptions.
136 *
137 * Examples: See \l Page
138+ *
139+ * \section2 Mnemonics
140+ * Since Ubuntu.Components 1.3 Action supports mnemonics. Mnemonics are shortcuts
141+ * defined in the \l text property, prefixed the shortcut letter with \&. For instance
142+ * \c "\&Call" will bint the \c "Alt-C" shortcut to the action. When a mnemonic
143+ * is detected on the Action and a keyboard is attached to the device, the \l text
144+ * property will provide a formatted text having the mnemonic letter underscored.
145+ * \qml
146+ * Action {
147+ * id: call
148+ * iconName: "call"
149+ * text: "&Call"
150+ * }
151+ * \endqml
152 */
153
154 /*!
155@@ -94,7 +133,69 @@
156 /*!
157 * \qmlproperty string Action::text
158 * The user visible primary label of the action.
159+ *
160+ * Mnemonics are shortcuts prefixed in the text with \&. If the text has multiple
161+ * occurences of the \& character, the first one will be considered for the shortcut.
162+ * The \& character cannot be used as shortcut.
163 */
164+QString UCAction::text()
165+{
166+ // if we have a mnemonic, underscore it
167+ if (!m_mnemonic.isEmpty()) {
168+
169+ QString mnemonic = "&" + m_mnemonic.toString().remove("Alt+");
170+ // patch special cases
171+ mnemonic.replace("Space", " ");
172+ int mnemonicIndex = m_text.indexOf(mnemonic);
173+ if (mnemonicIndex < 0) {
174+ // try lower case
175+ mnemonic = mnemonic.toLower();
176+ mnemonicIndex = m_text.indexOf(mnemonic);
177+ }
178+ QString displayText(m_text);
179+ // FIXME: we need QInputDeviceInfo to detect the keyboard attechment
180+ // https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1276808
181+ if (QuickUtils::instance().keyboardAttached()) {
182+ // underscore the character
183+ displayText.replace(mnemonicIndex, mnemonic.length(), "<u>" + mnemonic[1] + "</u>");
184+ } else {
185+ displayText.remove(mnemonicIndex, 1);
186+ }
187+ return displayText;
188+ }
189+ return m_text;
190+}
191+void UCAction::setText(const QString &text)
192+{
193+ if (m_text == text) {
194+ return;
195+ }
196+ m_text = text;
197+ setMnemonicFromText(m_text);
198+ Q_EMIT textChanged();
199+}
200+void UCAction::resetText()
201+{
202+ setText(QString());
203+}
204+
205+void UCAction::setMnemonicFromText(const QString &text)
206+{
207+ QKeySequence sequence = QKeySequence::mnemonic(text);
208+ if (sequence == m_mnemonic) {
209+ return;
210+ }
211+ if (!m_mnemonic.isEmpty()) {
212+ QGuiApplicationPrivate::instance()->shortcutMap.removeShortcut(0, this, m_mnemonic);
213+ }
214+
215+ m_mnemonic = sequence;
216+
217+ if (!m_mnemonic.isEmpty()) {
218+ Qt::ShortcutContext context = Qt::WindowShortcut;
219+ QGuiApplicationPrivate::instance()->shortcutMap.addShortcut(this, m_mnemonic, context, shortcutContextMatcher);
220+ }
221+}
222
223 /*!
224 * \qmlproperty string Action::keywords
225@@ -158,11 +259,16 @@
226 , m_published(false)
227 {
228 generateName();
229+ // FIXME: we need QInputDeviceInfo to detect the keyboard attechment
230+ // https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1276808
231+ connect(&QuickUtils::instance(), &QuickUtils::keyboardAttachedChanged,
232+ this, &UCAction::onKeyboardAttached);
233 }
234
235 UCAction::~UCAction()
236 {
237 resetShortcut();
238+ resetText();
239 }
240
241 bool UCAction::isValidType(QVariant::Type valueType)
242@@ -249,22 +355,6 @@
243 qWarning() << "Action.itemHint is a DEPRECATED property. Use ActionItems to specify the representation of an Action.";
244 }
245
246-bool shortcutContextMatcher(QObject* object, Qt::ShortcutContext)
247-{
248- UCAction* action = static_cast<UCAction*>(object);
249- // Can't access member here because it's not public
250- if (!action->property("enabled").toBool())
251- return false;
252-
253- QObject* window = object;
254- while (window && !window->isWindowType()) {
255- window = window->parent();
256- if (QQuickItem* item = qobject_cast<QQuickItem*>(window))
257- window = item->window();
258- }
259- return window && window == QGuiApplication::focusWindow();
260-}
261-
262 QKeySequence sequenceFromVariant(const QVariant& variant) {
263 if (variant.type() == QVariant::Int)
264 return static_cast<QKeySequence::StandardKey>(variant.toInt());
265@@ -320,6 +410,14 @@
266 return true;
267 }
268
269+// trigger text changes whenever HW keyboad is attached/detached
270+void UCAction::onKeyboardAttached()
271+{
272+ if (!m_mnemonic.isEmpty()) {
273+ Q_EMIT textChanged();
274+ }
275+}
276+
277 /*!
278 * \qmlmethod Action::trigger(var value)
279 * Checks the \c value against the action \l parameterType and triggers the action.
280
281=== modified file 'src/Ubuntu/Components/plugin/ucaction.h'
282--- src/Ubuntu/Components/plugin/ucaction.h 2015-12-14 06:15:16 +0000
283+++ src/Ubuntu/Components/plugin/ucaction.h 2015-12-18 15:21:18 +0000
284@@ -20,6 +20,7 @@
285 #include <QtCore/QObject>
286 #include <QtCore/QVariant>
287 #include <QtCore/QUrl>
288+#include <QtGui/QKeySequence>
289
290 // the function detects whether QML has an overridden trigger() slot available
291 // and invokes the one with the appropriate signature
292@@ -51,7 +52,7 @@
293 // transferred from Unity Actions
294 Q_ENUMS(Type)
295 Q_PROPERTY(QString name MEMBER m_name WRITE setName NOTIFY nameChanged)
296- Q_PROPERTY(QString text MEMBER m_text NOTIFY textChanged)
297+ Q_PROPERTY(QString text READ text WRITE setText RESET resetText NOTIFY textChanged)
298 Q_PROPERTY(QString iconName MEMBER m_iconName WRITE setIconName NOTIFY iconNameChanged)
299 Q_PROPERTY(QString description MEMBER m_description NOTIFY descriptionChanged)
300 Q_PROPERTY(QString keywords MEMBER m_keywords NOTIFY keywordsChanged)
301@@ -84,6 +85,9 @@
302 }
303
304 void setName(const QString &name);
305+ QString text();
306+ void setText(const QString &text);
307+ void resetText();
308 void setIconName(const QString &name);
309 void setIconSource(const QUrl &url);
310 void setItemHint(QQmlComponent *);
311@@ -114,6 +118,7 @@
312 QString m_description;
313 QString m_keywords;
314 QVariant m_shortcut;
315+ QKeySequence m_mnemonic;
316 QQmlComponent *m_itemHint;
317 Type m_parameterType;
318 bool m_factoryIconSource:1;
319@@ -130,7 +135,9 @@
320
321 bool isValidType(QVariant::Type valueType);
322 void generateName();
323+ void setMnemonicFromText(const QString &text);
324 bool event(QEvent *event);
325+ void onKeyboardAttached();
326 };
327
328 #endif // UCACTION_H
329
330=== modified file 'tests/unit_x11/tst_components/tst_shortcuts.qml'
331--- tests/unit_x11/tst_components/tst_shortcuts.qml 2015-12-06 11:48:53 +0000
332+++ tests/unit_x11/tst_components/tst_shortcuts.qml 2015-12-18 15:21:18 +0000
333@@ -46,6 +46,10 @@
334
335 function init() {
336 }
337+ function cleanup() {
338+ spy.clear();
339+ shortcutSpy.clear();
340+ }
341
342 SignalSpy {
343 id: spy
344@@ -102,6 +106,60 @@
345 shortcutSpy.clear();
346 action.shortcut = undefined;
347 shortcutSpy.wait(200);
348+ shortcutSpy.target = null;
349+ }
350+
351+ function test_mnemonic_data() {
352+ return [
353+ {tag: "HW keyboard, valid 'C&all'", kbd: true, text: "C&all", displayText: "C<u>a</u>ll", key: Qt.Key_A, xfail: false},
354+ {tag: "HW keyboard, valid '&Save & Exit", kbd: true, text: "&Save & Exit", displayText: "<u>S</u>ave & Exit", key: Qt.Key_S, xfail: false},
355+ {tag: "HW keyboard, valid 'Hide &Seek'", kbd: true, text: "Hide&Seek", displayText: "Hide<u>S</u>eek", key: Qt.Key_S, xfail: false},
356+ {tag: "HW keyboard, valid 'Save & Exit'", kbd: true, text: "Save & Exit", displayText: "Save <u> </u>Exit", key: Qt.Key_Space, xfail: false},
357+ {tag: "HW keyboard, valid 'Paste &&&Proceed'", kbd: true, text: "Paste &&&Proceed", displayText: "Paste &&<u>P</u>roceed", key: Qt.Key_P, xfail: false},
358+ {tag: "HW keyboard, valid 'Cut &$'", kbd: true, text: "Cut &$", displayText: "Cut <u>$</u>", key: Qt.Key_Dollar, xfail: false},
359+ {tag: "HW keyboard, valid 'At &@'", kbd: true, text: "At &@", displayText: "At <u>@</u>", key: Qt.Key_At, xfail: false},
360+ {tag: "HW keyboard, valid '&_'", kbd: true, text: "&_", displayText: "<u>_</u>", key: Qt.Key_Underscore, xfail: false},
361+
362+ {tag: "HW keyboard, invalid '&&Call'", kbd: true, text: "&&Call", displayText: "&&Call", key: Qt.Key_Asterisk, xfail: true},
363+ {tag: "HW keyboard, invalid 'Jump &'", kbd: true, text: "Jump &", displayText: "Jump &", key: Qt.Key_Asterisk, xfail: true},
364+ {tag: "HW keyboard, invalid '&&'", kbd: true, text: "&&", displayText: "&&", key: Qt.Key_Asterisk, xfail: true},
365+
366+ {tag: "no HW keyboard", kbd: false, text: "&Call", displayText: "Call", key: Qt.Key_C, xfail: false},
367+ ];
368+ }
369+ function test_mnemonic(data) {
370+ QuickUtils.keyboardAttached = data.kbd;
371+ action.text = data.text;
372+ if (!data.kbd && QuickUtils.keyboardAttached) {
373+ skip("Cannot test this case: " + data.tag);
374+ }
375+ compare(action.text, data.displayText);
376+ // shortcut
377+ keyClick(data.key, Qt.AltModifier);
378+ if (data.xfail) {
379+ expectFail(data.tag, "invalid mnemonic");
380+ }
381+ spy.wait(200);
382+ }
383+
384+ SignalSpy {
385+ id: textSpy
386+ target: action
387+ signalName: "textChanged"
388+ }
389+
390+ function test_mnemonic_displaytext() {
391+ QuickUtils.keyboardAttached = false;
392+ if (QuickUtils.keyboardAttached) {
393+ skip("the test needs to be able to detach the keyboard");
394+ }
395+ action.text = "&Call";
396+ textSpy.clear();
397+ QuickUtils.keyboardAttached = true;
398+ if (!QuickUtils.keyboardAttached) {
399+ skip("the test needs to be able to attach the keyboard");
400+ }
401+ textSpy.wait(200);
402 }
403 }
404 }

Subscribers

People subscribed via source and target branches