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

[Zsombor Egri]

Add mnemonics support to Action.

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1743
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2576/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1304/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1306/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1303/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2576/rebuild

review: Needs Fixing (continuous-integration)

[lp:~zsombi/ubuntu-ui-toolkit/mnemonics]

1744. By Zsombor Egri on 2015-12-10

prereq sync

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:1744
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2603/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1331/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1334/console
    FAILURE: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1330/console

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2603/rebuild

review: Needs Fixing (continuous-integration)

[lp:~zsombi/ubuntu-ui-toolkit/mnemonics]

1745. By Zsombor Egri on 2015-12-10

documentation updates, build fixes, test adjustments

1746. By Zsombor Egri on 2015-12-10

staging sync

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1746
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2604/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1332
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1335
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1335/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1331

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2604/rebuild

review: Approve (continuous-integration)

[Cris Dywan (kalikiana)]

- 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)]

> - 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...

[lp:~zsombi/ubuntu-ui-toolkit/mnemonics]

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

[Cris Dywan (kalikiana)]

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

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:1748
http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2671/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-amd64-ci/1396
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1397
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-armhf-ci/1397/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-vivid-i386-ci/1395

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-sdk-team-ubuntu-ui-toolkit-staging-ci/2671/rebuild

review: Approve (continuous-integration)

=== modified file 'examples/ubuntu-ui-toolkit-gallery/Buttons.qml'
--- examples/ubuntu-ui-toolkit-gallery/Buttons.qml	2015-06-19 05:14:00 +0000
+++ examples/ubuntu-ui-toolkit-gallery/Buttons.qml	2015-12-18 15:21:18 +0000
@@ -50,8 +50,8 @@
                 objectName: "button_color"
                 width: units.gu(20)
                 action: Action {
-                    text: i18n.tr("Call %1").arg(shortcut)
-                    shortcut: 'Ctrl+C'
+                    text: i18n.tr("C&all %1").arg(shortcut)
+                    shortcut: 'Ctrl+L'
                     property bool flipped
                     onTriggered: flipped = !flipped
                 }

=== modified file 'examples/ubuntu-ui-toolkit-gallery/MainPage.qml'
--- examples/ubuntu-ui-toolkit-gallery/MainPage.qml	2015-12-16 11:53:01 +0000
+++ examples/ubuntu-ui-toolkit-gallery/MainPage.qml	2015-12-18 15:21:18 +0000
@@ -66,6 +66,18 @@
                 iconName: "starred"
                 visible: !QuickUtils.mouseAttached
                 onTriggered: QuickUtils.mouseAttached = true
+            },
+            Action {
+                text: i18n.tr("Detach keyboard")
+                iconName: "non-starred"
+                visible: QuickUtils.keyboardAttached
+                onTriggered: QuickUtils.keyboardAttached = false
+            },
+            Action {
+                text: i18n.tr("Attach keyboard")
+                iconName: "starred"
+                visible: !QuickUtils.keyboardAttached
+                onTriggered: QuickUtils.keyboardAttached = true
             }
         ]
     }

=== modified file 'src/Ubuntu/Components/plugin/quickutils.cpp'
--- src/Ubuntu/Components/plugin/quickutils.cpp	2015-12-07 09:07:00 +0000
+++ src/Ubuntu/Components/plugin/quickutils.cpp	2015-12-18 15:21:18 +0000
@@ -33,7 +33,8 @@
 QuickUtils::QuickUtils(QObject *parent) :
     QObject(parent),
     m_rootView(0),
-    m_mouseAttached(false)
+    m_mouseAttached(false),
+    m_keyboardAttached(false)
 {
     QGuiApplication::instance()->installEventFilter(this);
     m_omitIM << "ibus" << "none" << "compose";

=== modified file 'src/Ubuntu/Components/plugin/quickutils.h'
--- src/Ubuntu/Components/plugin/quickutils.h	2015-11-05 13:41:35 +0000
+++ src/Ubuntu/Components/plugin/quickutils.h	2015-12-18 15:21:18 +0000
@@ -32,6 +32,7 @@
     Q_PROPERTY(QString inputMethodProvider READ inputMethodProvider)
     Q_PROPERTY(bool touchScreenAvailable READ touchScreenAvailable NOTIFY touchScreenAvailableChanged)
     Q_PROPERTY(bool mouseAttached MEMBER m_mouseAttached NOTIFY mouseAttachedChanged)
+    Q_PROPERTY(bool keyboardAttached MEMBER m_keyboardAttached NOTIFY keyboardAttachedChanged)
 public:
     static QuickUtils& instance()
     {
@@ -53,6 +54,10 @@
     {
         return m_mouseAttached;
     }
+    bool keyboardAttached()
+    {
+        return m_keyboardAttached;
+    }
 
 Q_SIGNALS:
     void rootObjectChanged();
@@ -60,6 +65,7 @@
     void deactivated();
     void touchScreenAvailableChanged();
     void mouseAttachedChanged();
+    void keyboardAttachedChanged();
 
 protected:
     bool eventFilter(QObject *, QEvent *);
@@ -69,6 +75,7 @@
     QPointer<QQuickView> m_rootView;
     QStringList m_omitIM;
     bool m_mouseAttached;
+    bool m_keyboardAttached;
 
     void lookupQuickView();
 };

=== modified file 'src/Ubuntu/Components/plugin/ucaction.cpp'
--- src/Ubuntu/Components/plugin/ucaction.cpp	2015-12-13 07:48:56 +0000
+++ src/Ubuntu/Components/plugin/ucaction.cpp	2015-12-18 15:21:18 +0000
@@ -15,6 +15,7 @@
  */
 
 #include "ucaction.h"
+#include "quickutils.h"
 
 #include <QtDebug>
 #include <QtQml/QQmlInfo>
@@ -22,6 +23,30 @@
 #include <QtQuick/qquickwindow.h>
 #include <private/qguiapplication_p.h>
 
+bool shortcutContextMatcher(QObject* object, Qt::ShortcutContext context)
+{
+    UCAction* action = static_cast<UCAction*>(object);
+    // Can't access member here because it's not public
+    if (!action->property("enabled").toBool())
+        return false;
+
+    switch (context) {
+    case Qt::ApplicationShortcut:
+        return true;
+    case Qt::WindowShortcut: {
+        QObject* window = object;
+        while (window && !window->isWindowType()) {
+            window = window->parent();
+            if (QQuickItem* item = qobject_cast<QQuickItem*>(window))
+                window = item->window();
+        }
+        return window && window == QGuiApplication::focusWindow();
+    }
+    default: break;
+    }
+    return false;
+}
+
 /*!
  * \qmltype Action
  * \instantiates UCAction
@@ -74,6 +99,20 @@
  * as well as to define actions for pages, or when defining options in \c ListItemOptions.
  *
  * Examples: See \l Page
+ *
+ * \section2 Mnemonics
+ * Since Ubuntu.Components 1.3 Action supports mnemonics. Mnemonics are shortcuts
+ * defined in the \l text property, prefixed the shortcut letter with \&. For instance
+ * \c "\&Call" will bint the \c "Alt-C" shortcut to the action. When a mnemonic
+ * is detected on the Action and a keyboard is attached to the device, the \l text
+ * property will provide a formatted text having the mnemonic letter underscored.
+ * \qml
+ * Action {
+ *     id: call
+ *     iconName: "call"
+ *     text: "&Call"
+ * }
+ * \endqml
  */
 
 /*!
@@ -94,7 +133,69 @@
 /*!
  * \qmlproperty string Action::text
  * The user visible primary label of the action.
+ *
+ * Mnemonics are shortcuts prefixed in the text with \&. If the text has multiple
+ * occurences of the \& character, the first one will be considered for the shortcut.
+ * The \& character cannot be used as shortcut.
  */
+QString UCAction::text()
+{
+    // if we have a mnemonic, underscore it
+    if (!m_mnemonic.isEmpty()) {
+
+        QString mnemonic = "&" + m_mnemonic.toString().remove("Alt+");
+        // patch special cases
+        mnemonic.replace("Space", " ");
+        int mnemonicIndex = m_text.indexOf(mnemonic);
+        if (mnemonicIndex < 0) {
+            // try lower case
+            mnemonic = mnemonic.toLower();
+            mnemonicIndex = m_text.indexOf(mnemonic);
+        }
+        QString displayText(m_text);
+        // FIXME: we need QInputDeviceInfo to detect the keyboard attechment
+        // https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1276808
+        if (QuickUtils::instance().keyboardAttached()) {
+            // underscore the character
+            displayText.replace(mnemonicIndex, mnemonic.length(), "<u>" + mnemonic[1] + "</u>");
+        } else {
+            displayText.remove(mnemonicIndex, 1);
+        }
+        return displayText;
+    }
+    return m_text;
+}
+void UCAction::setText(const QString &text)
+{
+    if (m_text == text) {
+        return;
+    }
+    m_text = text;
+    setMnemonicFromText(m_text);
+    Q_EMIT textChanged();
+}
+void UCAction::resetText()
+{
+    setText(QString());
+}
+
+void UCAction::setMnemonicFromText(const QString &text)
+{
+    QKeySequence sequence = QKeySequence::mnemonic(text);
+    if (sequence == m_mnemonic) {
+        return;
+    }
+    if (!m_mnemonic.isEmpty()) {
+        QGuiApplicationPrivate::instance()->shortcutMap.removeShortcut(0, this, m_mnemonic);
+    }
+
+    m_mnemonic = sequence;
+
+    if (!m_mnemonic.isEmpty()) {
+        Qt::ShortcutContext context = Qt::WindowShortcut;
+        QGuiApplicationPrivate::instance()->shortcutMap.addShortcut(this, m_mnemonic, context, shortcutContextMatcher);
+    }
+}
 
 /*!
  * \qmlproperty string Action::keywords
@@ -158,11 +259,16 @@
     , m_published(false)
 {
     generateName();
+    // FIXME: we need QInputDeviceInfo to detect the keyboard attechment
+    // https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1276808
+    connect(&QuickUtils::instance(), &QuickUtils::keyboardAttachedChanged,
+            this, &UCAction::onKeyboardAttached);
 }
 
 UCAction::~UCAction()
 {
     resetShortcut();
+    resetText();
 }
 
 bool UCAction::isValidType(QVariant::Type valueType)
@@ -249,22 +355,6 @@
     qWarning() << "Action.itemHint is a DEPRECATED property. Use ActionItems to specify the representation of an Action.";
 }
 
-bool shortcutContextMatcher(QObject* object, Qt::ShortcutContext)
-{
-    UCAction* action = static_cast<UCAction*>(object);
-    // Can't access member here because it's not public
-    if (!action->property("enabled").toBool())
-        return false;
-
-    QObject* window = object;
-    while (window && !window->isWindowType()) {
-        window = window->parent();
-        if (QQuickItem* item = qobject_cast<QQuickItem*>(window))
-            window = item->window();
-    }
-    return window && window == QGuiApplication::focusWindow();
-}
-
 QKeySequence sequenceFromVariant(const QVariant& variant) {
     if (variant.type() == QVariant::Int)
         return static_cast<QKeySequence::StandardKey>(variant.toInt());
@@ -320,6 +410,14 @@
     return true;
 }
 
+// trigger text changes whenever HW keyboad is attached/detached
+void UCAction::onKeyboardAttached()
+{
+    if (!m_mnemonic.isEmpty()) {
+        Q_EMIT textChanged();
+    }
+}
+
 /*!
  * \qmlmethod Action::trigger(var value)
  * Checks the \c value against the action \l parameterType and triggers the action.

=== modified file 'src/Ubuntu/Components/plugin/ucaction.h'
--- src/Ubuntu/Components/plugin/ucaction.h	2015-12-14 06:15:16 +0000
+++ src/Ubuntu/Components/plugin/ucaction.h	2015-12-18 15:21:18 +0000
@@ -20,6 +20,7 @@
 #include <QtCore/QObject>
 #include <QtCore/QVariant>
 #include <QtCore/QUrl>
+#include <QtGui/QKeySequence>
 
 // the function detects whether QML has an overridden trigger() slot available
 // and invokes the one with the appropriate signature
@@ -51,7 +52,7 @@
     // transferred from Unity Actions
     Q_ENUMS(Type)
     Q_PROPERTY(QString name MEMBER m_name WRITE setName NOTIFY nameChanged)
-    Q_PROPERTY(QString text MEMBER m_text NOTIFY textChanged)
+    Q_PROPERTY(QString text READ text WRITE setText RESET resetText NOTIFY textChanged)
     Q_PROPERTY(QString iconName MEMBER m_iconName WRITE setIconName NOTIFY iconNameChanged)
     Q_PROPERTY(QString description MEMBER m_description NOTIFY descriptionChanged)
     Q_PROPERTY(QString keywords MEMBER m_keywords NOTIFY keywordsChanged)
@@ -84,6 +85,9 @@
     }
 
     void setName(const QString &name);
+    QString text();
+    void setText(const QString &text);
+    void resetText();
     void setIconName(const QString &name);
     void setIconSource(const QUrl &url);
     void setItemHint(QQmlComponent *);
@@ -114,6 +118,7 @@
     QString m_description;
     QString m_keywords;
     QVariant m_shortcut;
+    QKeySequence m_mnemonic;
     QQmlComponent *m_itemHint;
     Type m_parameterType;
     bool m_factoryIconSource:1;
@@ -130,7 +135,9 @@
 
     bool isValidType(QVariant::Type valueType);
     void generateName();
+    void setMnemonicFromText(const QString &text);
     bool event(QEvent *event);
+    void onKeyboardAttached();
 };
 
 #endif // UCACTION_H

=== modified file 'tests/unit_x11/tst_components/tst_shortcuts.qml'
--- tests/unit_x11/tst_components/tst_shortcuts.qml	2015-12-06 11:48:53 +0000
+++ tests/unit_x11/tst_components/tst_shortcuts.qml	2015-12-18 15:21:18 +0000
@@ -46,6 +46,10 @@
 
         function init() {
         }
+        function cleanup() {
+            spy.clear();
+            shortcutSpy.clear();
+        }
 
         SignalSpy {
             id: spy
@@ -102,6 +106,60 @@
             shortcutSpy.clear();
             action.shortcut = undefined;
             shortcutSpy.wait(200);
+            shortcutSpy.target = null;
+        }
+
+        function test_mnemonic_data() {
+            return [
+                {tag: "HW keyboard, valid 'C&all'", kbd: true, text: "C&all", displayText: "C<u>a</u>ll", key: Qt.Key_A, xfail: false},
+                {tag: "HW keyboard, valid '&Save & Exit", kbd: true, text: "&Save & Exit", displayText: "<u>S</u>ave & Exit", key: Qt.Key_S, xfail: false},
+                {tag: "HW keyboard, valid 'Hide &Seek'", kbd: true, text: "Hide&Seek", displayText: "Hide<u>S</u>eek", key: Qt.Key_S, xfail: false},
+                {tag: "HW keyboard, valid 'Save & Exit'", kbd: true, text: "Save & Exit", displayText: "Save <u> </u>Exit", key: Qt.Key_Space, xfail: false},
+                {tag: "HW keyboard, valid 'Paste &&&Proceed'", kbd: true, text: "Paste &&&Proceed", displayText: "Paste &&<u>P</u>roceed", key: Qt.Key_P, xfail: false},
+                {tag: "HW keyboard, valid 'Cut &$'", kbd: true, text: "Cut &$", displayText: "Cut <u>$</u>", key: Qt.Key_Dollar, xfail: false},
+                {tag: "HW keyboard, valid 'At &@'", kbd: true, text: "At &@", displayText: "At <u>@</u>", key: Qt.Key_At, xfail: false},
+                {tag: "HW keyboard, valid '&_'", kbd: true, text: "&_", displayText: "<u>_</u>", key: Qt.Key_Underscore, xfail: false},
+
+                {tag: "HW keyboard, invalid '&&Call'", kbd: true, text: "&&Call", displayText: "&&Call", key: Qt.Key_Asterisk, xfail: true},
+                {tag: "HW keyboard, invalid 'Jump &'", kbd: true, text: "Jump &", displayText: "Jump &", key: Qt.Key_Asterisk, xfail: true},
+                {tag: "HW keyboard, invalid '&&'", kbd: true, text: "&&", displayText: "&&", key: Qt.Key_Asterisk, xfail: true},
+
+                {tag: "no HW keyboard", kbd: false, text: "&Call", displayText: "Call", key: Qt.Key_C, xfail: false},
+            ];
+        }
+        function test_mnemonic(data) {
+            QuickUtils.keyboardAttached = data.kbd;
+            action.text = data.text;
+            if (!data.kbd && QuickUtils.keyboardAttached) {
+                skip("Cannot test this case: " + data.tag);
+            }
+            compare(action.text, data.displayText);
+            // shortcut
+            keyClick(data.key, Qt.AltModifier);
+            if (data.xfail) {
+                expectFail(data.tag, "invalid mnemonic");
+            }
+            spy.wait(200);
+        }
+
+        SignalSpy {
+            id: textSpy
+            target: action
+            signalName: "textChanged"
+        }
+
+        function test_mnemonic_displaytext() {
+            QuickUtils.keyboardAttached = false;
+            if (QuickUtils.keyboardAttached) {
+                skip("the test needs to be able to detach the keyboard");
+            }
+            action.text = "&Call";
+            textSpy.clear();
+            QuickUtils.keyboardAttached = true;
+            if (!QuickUtils.keyboardAttached) {
+                skip("the test needs to be able to attach the keyboard");
+            }
+            textSpy.wait(200);
         }
     }
 }