Merge lp:~sil2100/ubuntu-keyboard/feedback_sound_gsettings into lp:ubuntu-keyboard

[Łukasz Zemczak]

Store the keyboard feedback sound in gsettings, making it configurable.

* Are there any related MPs required for this MP to build/function as expected? Please list.
-> No.

* Is your branch in sync with latest trunk (e.g. bzr pull lp:trunk -> no changes)
-> Yes.

Did you perform an exploratory manual test run of your code change and any related functionality on device or emulator?
-> Yes.

Did you successfully run all tests found in your component's Test Plan on device or emulator?
-> All testing has been performed on a Mako device.

If you changed the UI, was the change specified/approved by design?
-> N/A.

If you changed the packaging (debian), did you subscribe a core-dev to this MP?
-> N/A

Store the keyboard feedback sound in gsettings, making it configurable.

[Sebastien Bacher (seb128)]

Thanks Łukasz, one comment (without doing a full code review), could we have that path to be an absolute one rather than relative to some code defined directory? One of the goal is to be able to preview the sound in settings and it would be nicer to not have to code a directory there but just play the filename from the key

review: Needs Fixing

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:147
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/344/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-trusty-amd64-ci/173
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-trusty-armhf-ci/173
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-trusty-armhf-ci/173/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-trusty-i386-ci/172

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-keyboard-ci/344/rebuild

review: Approve (continuous-integration)

[Łukasz Zemczak (sil2100)]

Ok, let me try and change that, thanks for the pointer!

[lp:~sil2100/ubuntu-keyboard/feedback_sound_gsettings]

148. By Łukasz Zemczak on 2014-03-26

Try using absolute path for the feedback sound

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:148
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/345/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-trusty-amd64-ci/174
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-trusty-armhf-ci/174
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-trusty-armhf-ci/174/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-trusty-i386-ci/173

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-keyboard-ci/345/rebuild

review: Approve (continuous-integration)

[Sebastien Bacher (seb128)]

Could somebody from the ubuntu-keyboard team review that work? it has been pending review for a while

[lp:~sil2100/ubuntu-keyboard/feedback_sound_gsettings]

149. By Łukasz Zemczak on 2014-06-13

Merge trunk and resolve conflicts

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:149
http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-ci/403/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-amd64-ci/28
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/28
        deb: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-armhf-ci/28/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/ubuntu-keyboard-utopic-i386-ci/28

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/ubuntu-keyboard-ci/403/rebuild

review: Approve (continuous-integration)

[Michael Sheldon (michael-sheldon)]

Did you perform an exploratory manual test run of the code change and any related functionality on device or emulator?

 * Yes

Did CI run pass? If not, please explain why.

 * Yes

Have you checked that submitter has accurately filled out the submitter checklist and has taken no shortcut?

 * Yes

review: Approve

=== modified file 'data/schemas/com.canonical.keyboard.maliit.gschema.xml'
--- data/schemas/com.canonical.keyboard.maliit.gschema.xml	2014-05-12 20:24:36 +0000
+++ data/schemas/com.canonical.keyboard.maliit.gschema.xml	2014-06-13 12:25:24 +0000
@@ -31,6 +31,11 @@
       <description>Emit sound on key press.</description>
       <default>false</default>
     </key>
+    <key name="key-press-feedback-sound" type="s">
+      <summary>Key press feedback sound</summary>
+      <description>Path to the key press feedback sound.</description>
+      <default>'/usr/share/maliit/plugins/com/ubuntu/styles/ubuntu/sounds/key_tick2_quiet.wav'</default>
+    </key>
     <key name="key-press-haptic-feedback" type="b">
       <summary>Key press haptic feedback</summary>
       <description>Vibrate on key press.</description>

=== modified file 'qml/KeyboardContainer.qml'
--- qml/KeyboardContainer.qml	2014-05-20 15:04:34 +0000
+++ qml/KeyboardContainer.qml	2014-06-13 12:25:24 +0000
@@ -57,7 +57,12 @@
 
     SoundEffect {
         id: audioFeedback
-        source: Qt.resolvedUrl("styles/ubuntu/sounds/key_tick2_quiet.wav")
+        source: maliit_input_method.audioFeedbackSound
+    }
+
+    Connections {
+        target: maliit_input_method
+        onAudioFeedbackSoundChanged: audioFeedback.source = sound;
     }
 
     HapticsEffect {

=== modified file 'src/plugin/inputmethod.cpp'
--- src/plugin/inputmethod.cpp	2014-06-11 13:33:06 +0000
+++ src/plugin/inputmethod.cpp	2014-06-13 12:25:24 +0000
@@ -103,6 +103,7 @@
     connect(this, SIGNAL(contentTypeChanged(TextContentType)), this, SLOT(setContentType(TextContentType)));
     connect(this, SIGNAL(activeLanguageChanged(QString)), d->editor.wordEngine(), SLOT(onLanguageChanged(QString)));
     connect(d->m_geometry, SIGNAL(visibleRectChanged()), this, SLOT(onVisibleRectChanged()));
+    d->registerAudioFeedbackSoundSetting();
     d->registerAudioFeedbackSetting();
     d->registerHapticFeedbackSetting();
     d->registerAutoCorrectSetting();
@@ -450,6 +451,15 @@
     return d->actionKeyOverrider.data();
 }
 
+//! \brief InputMethod::audioFeedbackSound returns the current path to the audio
+//! feedback sound
+//! \return path to the feedback sound
+const QString InputMethod::audioFeedbackSound() const
+{
+    Q_D(const InputMethod);
+    return d->m_settings.keyPressAudioFeedbackSound();
+}
+
 //! \brief InputMethod::setActiveLanguage
 //! Sets the currently active/used language
 //! \param newLanguage id of the new language. For example "en" or "es"

=== modified file 'src/plugin/inputmethod.h'
--- src/plugin/inputmethod.h	2014-05-01 16:39:00 +0000
+++ src/plugin/inputmethod.h	2014-06-13 12:25:24 +0000
@@ -52,6 +52,7 @@
     Q_PROPERTY(QStringList enabledLanguages READ enabledLanguages NOTIFY enabledLanguagesChanged)
     Q_PROPERTY(QString activeLanguage READ activeLanguage WRITE setActiveLanguage NOTIFY activeLanguageChanged)
     Q_PROPERTY(bool useAudioFeedback READ useAudioFeedback NOTIFY useAudioFeedbackChanged)
+    Q_PROPERTY(QString audioFeedbackSound READ audioFeedbackSound NOTIFY audioFeedbackSoundChanged)
     Q_PROPERTY(QObject* actionKeyOverride READ actionKeyOverride NOTIFY actionKeyOverrideChanged)
     Q_PROPERTY(bool useHapticFeedback READ useHapticFeedback NOTIFY useHapticFeedbackChanged)
 
@@ -106,7 +107,9 @@
     Q_SLOT void setActiveLanguage(const QString& newLanguage);
 
     Q_SLOT void onVisibleRectChanged();
+
     bool useAudioFeedback() const;
+    const QString audioFeedbackSound() const;
     bool useHapticFeedback() const;
 
     QObject* actionKeyOverride() const;
@@ -117,6 +120,7 @@
     void enabledLanguagesChanged(QStringList languages);
     void activeLanguageChanged(QString language);
     void useAudioFeedbackChanged();
+    void audioFeedbackSoundChanged(QString sound);
     void useHapticFeedbackChanged();
     void wordEngineEnabledChanged(bool wordEngineEnabled);
     void wordRibbonEnabledChanged(bool wordRibbonEnabled);

=== modified file 'src/plugin/inputmethod_p.h'
--- src/plugin/inputmethod_p.h	2014-05-12 15:30:05 +0000
+++ src/plugin/inputmethod_p.h	2014-06-13 12:25:24 +0000
@@ -178,6 +178,12 @@
     /*
      * register settings
      */
+    void registerAudioFeedbackSoundSetting()
+    {
+        QObject::connect(&m_settings, SIGNAL(keyPressAudioFeedbackSoundChanged(QString)),
+                         q, SIGNAL(audioFeedbackSoundChanged(QString)));
+    }
+
     void registerAudioFeedbackSetting()
     {
         QObject::connect(&m_settings, SIGNAL(keyPressAudioFeedbackChanged(bool)),

=== modified file 'src/plugin/keyboardsettings.cpp'
--- src/plugin/keyboardsettings.cpp	2014-04-17 11:16:56 +0000
+++ src/plugin/keyboardsettings.cpp	2014-06-13 12:25:24 +0000
@@ -41,6 +41,7 @@
 const QLatin1String PREDICTIVE_TEXT_KEY = QLatin1String("predictiveText");
 const QLatin1String SPELL_CHECKING_KEY = QLatin1String("spellChecking");
 const QLatin1String KEY_PRESS_AUDIO_FEEDBACK_KEY = QLatin1String("keyPressFeedback");
+const QLatin1String KEY_PRESS_AUDIO_FEEDBACK_SOUND_KEY = QLatin1String("keyPressFeedbackSound");
 const QLatin1String KEY_PRESS_HAPTIC_FEEDBACK_KEY = QLatin1String("keyPressHapticFeedback");
 
 /*!
@@ -142,6 +143,16 @@
 }
 
 /*!
+ * \brief KeyboardSettings::keyPressFeedbackSound returns the path to the current key
+ * feedback sound
+ * \return path to the feedback sound
+ */
+QString KeyboardSettings::keyPressAudioFeedbackSound() const
+{
+    return m_settings->get(KEY_PRESS_AUDIO_FEEDBACK_SOUND_KEY).toString();
+}
+
+/*!
  * \brief KeyboardSettings::settingUpdated slot to handle changes in the settings backend
  * A specialized signal is emitted for the affected setting
  * \param key
@@ -172,6 +183,9 @@
     } else if (key == KEY_PRESS_HAPTIC_FEEDBACK_KEY) {
         Q_EMIT keyPressHapticFeedbackChanged(keyPressHapticFeedback());
         return;
+    } else if (key == KEY_PRESS_AUDIO_FEEDBACK_SOUND_KEY) {
+        Q_EMIT keyPressAudioFeedbackSoundChanged(keyPressAudioFeedbackSound());
+        return;
     }
 
     qWarning() << Q_FUNC_INFO << "unknown settings key:" << key;

=== modified file 'src/plugin/keyboardsettings.h'
--- src/plugin/keyboardsettings.h	2014-04-17 11:16:56 +0000
+++ src/plugin/keyboardsettings.h	2014-06-13 12:25:24 +0000
@@ -51,6 +51,7 @@
     bool predictiveText() const;
     bool spellchecking() const;
     bool keyPressAudioFeedback() const;
+    QString keyPressAudioFeedbackSound() const;
     bool keyPressHapticFeedback() const;
 
 Q_SIGNALS:
@@ -61,6 +62,7 @@
     void predictiveTextChanged(bool);
     void spellCheckingChanged(bool);
     void keyPressAudioFeedbackChanged(bool);
+    void keyPressAudioFeedbackSoundChanged(QString);
     void keyPressHapticFeedbackChanged(bool);
 
 private:

=== modified file 'tests/unittests/ut_keyboardsettings/ut_keyboardsettings.cpp'
--- tests/unittests/ut_keyboardsettings/ut_keyboardsettings.cpp	2014-04-17 11:16:56 +0000
+++ tests/unittests/ut_keyboardsettings/ut_keyboardsettings.cpp	2014-06-13 12:25:24 +0000
@@ -65,21 +65,24 @@
         QTest::addColumn<int>("predictSpyCount");
         QTest::addColumn<int>("spellSpyCount");
         QTest::addColumn<int>("feedbackSpyCount");
+        QTest::addColumn<int>("feedbackSoundSpyCount");
 
         QTest::newRow("languages changed") << QString("enabledLanguages")
-                                           << 1 << 0 << 0 << 0 << 0 << 0;
+                                           << 1 << 0 << 0 << 0 << 0 << 0 << 0;
         QTest::newRow("capitalization changed") << QString("autoCapitalization")
-                                                << 0 << 1 << 0 << 0 << 0 << 0;
+                                                << 0 << 1 << 0 << 0 << 0 << 0 << 0;
         QTest::newRow("completion changed") << QString("autoCompletion")
-                                            << 0 << 0 << 1 << 0 << 0 << 0;
+                                            << 0 << 0 << 1 << 0 << 0 << 0 << 0;
         QTest::newRow("predict changed") << QString("predictiveText")
-                                         << 0 << 0 << 0 << 1 << 0 << 0;
+                                         << 0 << 0 << 0 << 1 << 0 << 0 << 0;
         QTest::newRow("spellcheck changed") << QString("spellChecking")
-                                         << 0 << 0 << 0 << 0 << 1 << 0;
+                                         << 0 << 0 << 0 << 0 << 1 << 0 << 0;
         QTest::newRow("feedback changed") << QString("keyPressFeedback")
-                                          << 0 << 0 << 0 << 0 << 0 << 1;
+                                          << 0 << 0 << 0 << 0 << 0 << 1 << 0;
+        QTest::newRow("feedback sound changed") << QString("keyPressFeedbackSound")
+                                          << 0 << 0 << 0 << 0 << 0 << 0 << 1;
         QTest::newRow("unknown changed") << QString("unknownKey")
-                                         << 0 << 0 << 0 << 0 << 0 << 0;
+                                         << 0 << 0 << 0 << 0 << 0 << 0 << 0;
     }
 
     Q_SLOT void testSettingUpdated()
@@ -91,6 +94,7 @@
         QFETCH(int, predictSpyCount);
         QFETCH(int, spellSpyCount);
         QFETCH(int, feedbackSpyCount);
+        QFETCH(int, feedbackSoundSpyCount);
 
         QSignalSpy languagesSpy(m_settings, SIGNAL(enabledLanguagesChanged(QStringList)));
         QSignalSpy capitalSpy(m_settings, SIGNAL(autoCapitalizationChanged(bool)));
@@ -98,6 +102,7 @@
         QSignalSpy predictSpy(m_settings, SIGNAL(predictiveTextChanged(bool)));
         QSignalSpy spellSpy(m_settings, SIGNAL(spellCheckingChanged(bool)));
         QSignalSpy feedbackSpy(m_settings, SIGNAL(keyPressAudioFeedbackChanged(bool)));
+        QSignalSpy feedbackSoundSpy(m_settings, SIGNAL(keyPressAudioFeedbackSoundChanged(QString)));
 
         m_settings->settingUpdated(key);
 
@@ -107,6 +112,7 @@
         QCOMPARE(predictSpy.count(), predictSpyCount);
         QCOMPARE(spellSpy.count(), spellSpyCount);
         QCOMPARE(feedbackSpy.count(), feedbackSpyCount);
+        QCOMPARE(feedbackSoundSpy.count(), feedbackSoundSpyCount);
     }
 };