ubuntu-keyboard

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

  1. feedback_sound_gsettings
  2. Merge into trunk
Proposed by Łukasz Zemczak
Status: Merged
Approved by: Michael Sheldon
Approved revision: 149
Merged at revision: 177
Proposed branch: lp:~sil2100/ubuntu-keyboard/feedback_sound_gsettings
Merge into: lp:ubuntu-keyboard
Diff against target: 226 lines (+60/-8)
8 files modified
data/schemas/com.canonical.keyboard.maliit.gschema.xml (+5/-0)
qml/KeyboardContainer.qml (+6/-1)
src/plugin/inputmethod.cpp (+10/-0)
src/plugin/inputmethod.h (+4/-0)
src/plugin/inputmethod_p.h (+6/-0)
src/plugin/keyboardsettings.cpp (+14/-0)
src/plugin/keyboardsettings.h (+2/-0)
tests/unittests/ut_keyboardsettings/ut_keyboardsettings.cpp (+13/-7)
To merge this branch: bzr merge lp:~sil2100/ubuntu-keyboard/feedback_sound_gsettings
Reviewer Review Type Date Requested Status
Michael Sheldon (community) Approve
PS Jenkins bot continuous-integration Approve
Sebastien Bacher Needs Fixing
Review via email: mp+212684@code.launchpad.net

Commit message

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

Description of the change

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

To post a comment you must log in.
Revision history for this message
Sebastien Bacher (seb128) wrote :

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
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Łukasz Zemczak (sil2100) wrote :

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

lp:~sil2100/ubuntu-keyboard/feedback_sound_gsettings updated
148. By Łukasz Zemczak

Try using absolute path for the feedback sound

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Sebastien Bacher (seb128) wrote :

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 updated
149. By Łukasz Zemczak

Merge trunk and resolve conflicts

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

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)
Revision history for this message
Michael Sheldon (michael-sheldon) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
31 <description>Emit sound on key press.</description>31 <description>Emit sound on key press.</description>
32 <default>false</default>32 <default>false</default>
33 </key>33 </key>
34 <key name="key-press-feedback-sound" type="s">
35 <summary>Key press feedback sound</summary>
36 <description>Path to the key press feedback sound.</description>
37 <default>'/usr/share/maliit/plugins/com/ubuntu/styles/ubuntu/sounds/key_tick2_quiet.wav'</default>
38 </key>
34 <key name="key-press-haptic-feedback" type="b">39 <key name="key-press-haptic-feedback" type="b">
35 <summary>Key press haptic feedback</summary>40 <summary>Key press haptic feedback</summary>
36 <description>Vibrate on key press.</description>41 <description>Vibrate on key press.</description>
3742
=== 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 @@
5757
58 SoundEffect {58 SoundEffect {
59 id: audioFeedback59 id: audioFeedback
60 source: Qt.resolvedUrl("styles/ubuntu/sounds/key_tick2_quiet.wav")60 source: maliit_input_method.audioFeedbackSound
61 }
62
63 Connections {
64 target: maliit_input_method
65 onAudioFeedbackSoundChanged: audioFeedback.source = sound;
61 }66 }
6267
63 HapticsEffect {68 HapticsEffect {
6469
=== 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 @@
103 connect(this, SIGNAL(contentTypeChanged(TextContentType)), this, SLOT(setContentType(TextContentType)));103 connect(this, SIGNAL(contentTypeChanged(TextContentType)), this, SLOT(setContentType(TextContentType)));
104 connect(this, SIGNAL(activeLanguageChanged(QString)), d->editor.wordEngine(), SLOT(onLanguageChanged(QString)));104 connect(this, SIGNAL(activeLanguageChanged(QString)), d->editor.wordEngine(), SLOT(onLanguageChanged(QString)));
105 connect(d->m_geometry, SIGNAL(visibleRectChanged()), this, SLOT(onVisibleRectChanged()));105 connect(d->m_geometry, SIGNAL(visibleRectChanged()), this, SLOT(onVisibleRectChanged()));
106 d->registerAudioFeedbackSoundSetting();
106 d->registerAudioFeedbackSetting();107 d->registerAudioFeedbackSetting();
107 d->registerHapticFeedbackSetting();108 d->registerHapticFeedbackSetting();
108 d->registerAutoCorrectSetting();109 d->registerAutoCorrectSetting();
@@ -450,6 +451,15 @@
450 return d->actionKeyOverrider.data();451 return d->actionKeyOverrider.data();
451}452}
452453
454//! \brief InputMethod::audioFeedbackSound returns the current path to the audio
455//! feedback sound
456//! \return path to the feedback sound
457const QString InputMethod::audioFeedbackSound() const
458{
459 Q_D(const InputMethod);
460 return d->m_settings.keyPressAudioFeedbackSound();
461}
462
453//! \brief InputMethod::setActiveLanguage463//! \brief InputMethod::setActiveLanguage
454//! Sets the currently active/used language464//! Sets the currently active/used language
455//! \param newLanguage id of the new language. For example "en" or "es"465//! \param newLanguage id of the new language. For example "en" or "es"
456466
=== 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 @@
52 Q_PROPERTY(QStringList enabledLanguages READ enabledLanguages NOTIFY enabledLanguagesChanged)52 Q_PROPERTY(QStringList enabledLanguages READ enabledLanguages NOTIFY enabledLanguagesChanged)
53 Q_PROPERTY(QString activeLanguage READ activeLanguage WRITE setActiveLanguage NOTIFY activeLanguageChanged)53 Q_PROPERTY(QString activeLanguage READ activeLanguage WRITE setActiveLanguage NOTIFY activeLanguageChanged)
54 Q_PROPERTY(bool useAudioFeedback READ useAudioFeedback NOTIFY useAudioFeedbackChanged)54 Q_PROPERTY(bool useAudioFeedback READ useAudioFeedback NOTIFY useAudioFeedbackChanged)
55 Q_PROPERTY(QString audioFeedbackSound READ audioFeedbackSound NOTIFY audioFeedbackSoundChanged)
55 Q_PROPERTY(QObject* actionKeyOverride READ actionKeyOverride NOTIFY actionKeyOverrideChanged)56 Q_PROPERTY(QObject* actionKeyOverride READ actionKeyOverride NOTIFY actionKeyOverrideChanged)
56 Q_PROPERTY(bool useHapticFeedback READ useHapticFeedback NOTIFY useHapticFeedbackChanged)57 Q_PROPERTY(bool useHapticFeedback READ useHapticFeedback NOTIFY useHapticFeedbackChanged)
5758
@@ -106,7 +107,9 @@
106 Q_SLOT void setActiveLanguage(const QString& newLanguage);107 Q_SLOT void setActiveLanguage(const QString& newLanguage);
107108
108 Q_SLOT void onVisibleRectChanged();109 Q_SLOT void onVisibleRectChanged();
110
109 bool useAudioFeedback() const;111 bool useAudioFeedback() const;
112 const QString audioFeedbackSound() const;
110 bool useHapticFeedback() const;113 bool useHapticFeedback() const;
111114
112 QObject* actionKeyOverride() const;115 QObject* actionKeyOverride() const;
@@ -117,6 +120,7 @@
117 void enabledLanguagesChanged(QStringList languages);120 void enabledLanguagesChanged(QStringList languages);
118 void activeLanguageChanged(QString language);121 void activeLanguageChanged(QString language);
119 void useAudioFeedbackChanged();122 void useAudioFeedbackChanged();
123 void audioFeedbackSoundChanged(QString sound);
120 void useHapticFeedbackChanged();124 void useHapticFeedbackChanged();
121 void wordEngineEnabledChanged(bool wordEngineEnabled);125 void wordEngineEnabledChanged(bool wordEngineEnabled);
122 void wordRibbonEnabledChanged(bool wordRibbonEnabled);126 void wordRibbonEnabledChanged(bool wordRibbonEnabled);
123127
=== 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 @@
178 /*178 /*
179 * register settings179 * register settings
180 */180 */
181 void registerAudioFeedbackSoundSetting()
182 {
183 QObject::connect(&m_settings, SIGNAL(keyPressAudioFeedbackSoundChanged(QString)),
184 q, SIGNAL(audioFeedbackSoundChanged(QString)));
185 }
186
181 void registerAudioFeedbackSetting()187 void registerAudioFeedbackSetting()
182 {188 {
183 QObject::connect(&m_settings, SIGNAL(keyPressAudioFeedbackChanged(bool)),189 QObject::connect(&m_settings, SIGNAL(keyPressAudioFeedbackChanged(bool)),
184190
=== 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 @@
41const QLatin1String PREDICTIVE_TEXT_KEY = QLatin1String("predictiveText");41const QLatin1String PREDICTIVE_TEXT_KEY = QLatin1String("predictiveText");
42const QLatin1String SPELL_CHECKING_KEY = QLatin1String("spellChecking");42const QLatin1String SPELL_CHECKING_KEY = QLatin1String("spellChecking");
43const QLatin1String KEY_PRESS_AUDIO_FEEDBACK_KEY = QLatin1String("keyPressFeedback");43const QLatin1String KEY_PRESS_AUDIO_FEEDBACK_KEY = QLatin1String("keyPressFeedback");
44const QLatin1String KEY_PRESS_AUDIO_FEEDBACK_SOUND_KEY = QLatin1String("keyPressFeedbackSound");
44const QLatin1String KEY_PRESS_HAPTIC_FEEDBACK_KEY = QLatin1String("keyPressHapticFeedback");45const QLatin1String KEY_PRESS_HAPTIC_FEEDBACK_KEY = QLatin1String("keyPressHapticFeedback");
4546
46/*!47/*!
@@ -142,6 +143,16 @@
142}143}
143144
144/*!145/*!
146 * \brief KeyboardSettings::keyPressFeedbackSound returns the path to the current key
147 * feedback sound
148 * \return path to the feedback sound
149 */
150QString KeyboardSettings::keyPressAudioFeedbackSound() const
151{
152 return m_settings->get(KEY_PRESS_AUDIO_FEEDBACK_SOUND_KEY).toString();
153}
154
155/*!
145 * \brief KeyboardSettings::settingUpdated slot to handle changes in the settings backend156 * \brief KeyboardSettings::settingUpdated slot to handle changes in the settings backend
146 * A specialized signal is emitted for the affected setting157 * A specialized signal is emitted for the affected setting
147 * \param key158 * \param key
@@ -172,6 +183,9 @@
172 } else if (key == KEY_PRESS_HAPTIC_FEEDBACK_KEY) {183 } else if (key == KEY_PRESS_HAPTIC_FEEDBACK_KEY) {
173 Q_EMIT keyPressHapticFeedbackChanged(keyPressHapticFeedback());184 Q_EMIT keyPressHapticFeedbackChanged(keyPressHapticFeedback());
174 return;185 return;
186 } else if (key == KEY_PRESS_AUDIO_FEEDBACK_SOUND_KEY) {
187 Q_EMIT keyPressAudioFeedbackSoundChanged(keyPressAudioFeedbackSound());
188 return;
175 }189 }
176190
177 qWarning() << Q_FUNC_INFO << "unknown settings key:" << key;191 qWarning() << Q_FUNC_INFO << "unknown settings key:" << key;
178192
=== 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 @@
51 bool predictiveText() const;51 bool predictiveText() const;
52 bool spellchecking() const;52 bool spellchecking() const;
53 bool keyPressAudioFeedback() const;53 bool keyPressAudioFeedback() const;
54 QString keyPressAudioFeedbackSound() const;
54 bool keyPressHapticFeedback() const;55 bool keyPressHapticFeedback() const;
5556
56Q_SIGNALS:57Q_SIGNALS:
@@ -61,6 +62,7 @@
61 void predictiveTextChanged(bool);62 void predictiveTextChanged(bool);
62 void spellCheckingChanged(bool);63 void spellCheckingChanged(bool);
63 void keyPressAudioFeedbackChanged(bool);64 void keyPressAudioFeedbackChanged(bool);
65 void keyPressAudioFeedbackSoundChanged(QString);
64 void keyPressHapticFeedbackChanged(bool);66 void keyPressHapticFeedbackChanged(bool);
6567
66private:68private:
6769
=== 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 @@
65 QTest::addColumn<int>("predictSpyCount");65 QTest::addColumn<int>("predictSpyCount");
66 QTest::addColumn<int>("spellSpyCount");66 QTest::addColumn<int>("spellSpyCount");
67 QTest::addColumn<int>("feedbackSpyCount");67 QTest::addColumn<int>("feedbackSpyCount");
68 QTest::addColumn<int>("feedbackSoundSpyCount");
6869
69 QTest::newRow("languages changed") << QString("enabledLanguages")70 QTest::newRow("languages changed") << QString("enabledLanguages")
70 << 1 << 0 << 0 << 0 << 0 << 0;71 << 1 << 0 << 0 << 0 << 0 << 0 << 0;
71 QTest::newRow("capitalization changed") << QString("autoCapitalization")72 QTest::newRow("capitalization changed") << QString("autoCapitalization")
72 << 0 << 1 << 0 << 0 << 0 << 0;73 << 0 << 1 << 0 << 0 << 0 << 0 << 0;
73 QTest::newRow("completion changed") << QString("autoCompletion")74 QTest::newRow("completion changed") << QString("autoCompletion")
74 << 0 << 0 << 1 << 0 << 0 << 0;75 << 0 << 0 << 1 << 0 << 0 << 0 << 0;
75 QTest::newRow("predict changed") << QString("predictiveText")76 QTest::newRow("predict changed") << QString("predictiveText")
76 << 0 << 0 << 0 << 1 << 0 << 0;77 << 0 << 0 << 0 << 1 << 0 << 0 << 0;
77 QTest::newRow("spellcheck changed") << QString("spellChecking")78 QTest::newRow("spellcheck changed") << QString("spellChecking")
78 << 0 << 0 << 0 << 0 << 1 << 0;79 << 0 << 0 << 0 << 0 << 1 << 0 << 0;
79 QTest::newRow("feedback changed") << QString("keyPressFeedback")80 QTest::newRow("feedback changed") << QString("keyPressFeedback")
80 << 0 << 0 << 0 << 0 << 0 << 1;81 << 0 << 0 << 0 << 0 << 0 << 1 << 0;
82 QTest::newRow("feedback sound changed") << QString("keyPressFeedbackSound")
83 << 0 << 0 << 0 << 0 << 0 << 0 << 1;
81 QTest::newRow("unknown changed") << QString("unknownKey")84 QTest::newRow("unknown changed") << QString("unknownKey")
82 << 0 << 0 << 0 << 0 << 0 << 0;85 << 0 << 0 << 0 << 0 << 0 << 0 << 0;
83 }86 }
8487
85 Q_SLOT void testSettingUpdated()88 Q_SLOT void testSettingUpdated()
@@ -91,6 +94,7 @@
91 QFETCH(int, predictSpyCount);94 QFETCH(int, predictSpyCount);
92 QFETCH(int, spellSpyCount);95 QFETCH(int, spellSpyCount);
93 QFETCH(int, feedbackSpyCount);96 QFETCH(int, feedbackSpyCount);
97 QFETCH(int, feedbackSoundSpyCount);
9498
95 QSignalSpy languagesSpy(m_settings, SIGNAL(enabledLanguagesChanged(QStringList)));99 QSignalSpy languagesSpy(m_settings, SIGNAL(enabledLanguagesChanged(QStringList)));
96 QSignalSpy capitalSpy(m_settings, SIGNAL(autoCapitalizationChanged(bool)));100 QSignalSpy capitalSpy(m_settings, SIGNAL(autoCapitalizationChanged(bool)));
@@ -98,6 +102,7 @@
98 QSignalSpy predictSpy(m_settings, SIGNAL(predictiveTextChanged(bool)));102 QSignalSpy predictSpy(m_settings, SIGNAL(predictiveTextChanged(bool)));
99 QSignalSpy spellSpy(m_settings, SIGNAL(spellCheckingChanged(bool)));103 QSignalSpy spellSpy(m_settings, SIGNAL(spellCheckingChanged(bool)));
100 QSignalSpy feedbackSpy(m_settings, SIGNAL(keyPressAudioFeedbackChanged(bool)));104 QSignalSpy feedbackSpy(m_settings, SIGNAL(keyPressAudioFeedbackChanged(bool)));
105 QSignalSpy feedbackSoundSpy(m_settings, SIGNAL(keyPressAudioFeedbackSoundChanged(QString)));
101106
102 m_settings->settingUpdated(key);107 m_settings->settingUpdated(key);
103108
@@ -107,6 +112,7 @@
107 QCOMPARE(predictSpy.count(), predictSpyCount);112 QCOMPARE(predictSpy.count(), predictSpyCount);
108 QCOMPARE(spellSpy.count(), spellSpyCount);113 QCOMPARE(spellSpy.count(), spellSpyCount);
109 QCOMPARE(feedbackSpy.count(), feedbackSpyCount);114 QCOMPARE(feedbackSpy.count(), feedbackSpyCount);
115 QCOMPARE(feedbackSoundSpy.count(), feedbackSoundSpyCount);
110 }116 }
111};117};
112118

Subscribers

People subscribed via source and target branches