Merge lp:~nick-dedekind/ubuntu-settings-components/lp1396058-multi-line-messages into lp:~registry/ubuntu-settings-components/trunk

Proposed by Nick Dedekind
Status: Merged
Approved by: Andrea Cimitan
Approved revision: 89
Merged at revision: 90
Proposed branch: lp:~nick-dedekind/ubuntu-settings-components/lp1396058-multi-line-messages
Merge into: lp:~registry/ubuntu-settings-components/trunk
Diff against target: 254 lines (+106/-23)
6 files modified
examples/MessageComponents.qml (+2/-0)
plugins/Ubuntu/Settings/Components/ActionTextField.qml (+4/-6)
plugins/Ubuntu/Settings/Menus/SnapDecisionMenu.qml (+5/-6)
plugins/Ubuntu/Settings/Menus/TextMessageMenu.qml (+0/-1)
tests/qmltests/Menus/tst_SnapDecisionMenu.qml (+51/-5)
tests/qmltests/Menus/tst_TextMessageMenu.qml (+44/-5)
To merge this branch: bzr merge lp:~nick-dedekind/ubuntu-settings-components/lp1396058-multi-line-messages
Reviewer Review Type Date Requested Status
Andrea Cimitan (community) Approve
PS Jenkins bot (community) continuous-integration Approve
Review via email: mp+257050@code.launchpad.net

Commit message

Multiline support for message replies (lp#1396058)

Description of the change

Fix for lp#1396058. Use TextArea rather than TextField

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

 * Did you perform an exploratory manual test run of your code change and any related functionality?
Yes

 * If you changed the packaging (debian), did you subscribe the ubuntu-unity team to this MP?
N/A

 * If you changed the UI, has there been a design review?
Yes

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Andrea Cimitan (cimi) wrote :

It looks like the textarea would like to be taller of a couple of pixels: now it is matched with the height of the button (looks nice) but adds a small scrollbar at the right of the widget. Could you figure out how we can get rid of this scrollbar when only one line is present?

review: Needs Fixing
Revision history for this message
Nick Dedekind (nick-dedekind) wrote :

> It looks like the textarea would like to be taller of a couple of pixels: now
> it is matched with the height of the button (looks nice) but adds a small
> scrollbar at the right of the widget. Could you figure out how we can get rid
> of this scrollbar when only one line is present?

I've tried this with an autoSizing TextArea on it's own. Happens there as well. SDK bug; please raise with them.

Revision history for this message
Andrea Cimitan (cimi) wrote :

 * Did you perform an exploratory manual test run of the code change and any related functionality?
Yes
 * Did CI run pass? If not, please explain why.
Yes

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'examples/MessageComponents.qml'
--- examples/MessageComponents.qml 2014-09-11 13:30:09 +0000
+++ examples/MessageComponents.qml 2015-04-22 10:48:20 +0000
@@ -92,6 +92,7 @@
92 body: model.body92 body: model.body
93 time: model.time93 time: model.time
94 removable: true94 removable: true
95 replyHintText: "Reply"
9596
96 onTriggered: {97 onTriggered: {
97 selected = !selected;98 selected = !selected;
@@ -108,6 +109,7 @@
108 body: model.body109 body: model.body
109 time: model.time110 time: model.time
110 removable: true111 removable: true
112 replyHintText: "Reply"
111113
112 onTriggered: {114 onTriggered: {
113 selected = !selected;115 selected = !selected;
114116
=== modified file 'plugins/Ubuntu/Settings/Components/ActionTextField.qml'
--- plugins/Ubuntu/Settings/Components/ActionTextField.qml 2014-11-12 10:49:22 +0000
+++ plugins/Ubuntu/Settings/Components/ActionTextField.qml 2015-04-22 10:48:20 +0000
@@ -33,7 +33,7 @@
3333
34 implicitHeight: layout.implicitHeight34 implicitHeight: layout.implicitHeight
3535
36 RowLayout {36 Row {
37 id: layout37 id: layout
38 anchors {38 anchors {
39 left: parent.left39 left: parent.left
@@ -41,13 +41,11 @@
41 }41 }
42 spacing: units.gu(1)42 spacing: units.gu(1)
4343
44 TextField {44 TextArea {
45 id: replyField45 id: replyField
46 objectName: "replyText"46 objectName: "replyText"
4747 autoSize: true
48 hasClearButton: false48 width: parent.width - layout.spacing - sendButton.width
49
50 Layout.fillWidth: true
5149
52 onEnabledChanged: {50 onEnabledChanged: {
53 //Make sure that the component lost focus when enabled = false,51 //Make sure that the component lost focus when enabled = false,
5452
=== modified file 'plugins/Ubuntu/Settings/Menus/SnapDecisionMenu.qml'
--- plugins/Ubuntu/Settings/Menus/SnapDecisionMenu.qml 2014-08-14 11:19:43 +0000
+++ plugins/Ubuntu/Settings/Menus/SnapDecisionMenu.qml 2015-04-22 10:48:20 +0000
@@ -31,6 +31,8 @@
3131
32 property bool replyEnabled: true32 property bool replyEnabled: true
33 property string replyButtonText: i18n.tr("Send")33 property string replyButtonText: i18n.tr("Send")
34 property string replyHintText
35 property bool replyExpanded: false
3436
35 signal actionActivated37 signal actionActivated
36 signal replied(string value)38 signal replied(string value)
@@ -57,11 +59,7 @@
57 Layout.fillWidth: true59 Layout.fillWidth: true
5860
59 onClicked: {61 onClicked: {
60 if (reply.state === "") {62 menu.replyExpanded = !menu.replyExpanded;
61 reply.state = "expanded";
62 } else {
63 reply.state = "";
64 }
65 }63 }
66 }64 }
6765
@@ -84,10 +82,11 @@
8482
85 Layout.fillWidth: true83 Layout.fillWidth: true
86 Layout.fillHeight: true84 Layout.fillHeight: true
87 visible: state == "expanded"85 visible: menu.replyExpanded
8886
89 activateEnabled: menu.replyEnabled87 activateEnabled: menu.replyEnabled
90 buttonText: menu.replyButtonText88 buttonText: menu.replyButtonText
89 textHint: menu.replyHintText
9190
92 onActivated: {91 onActivated: {
93 menu.replied(value);92 menu.replied(value);
9493
=== modified file 'plugins/Ubuntu/Settings/Menus/TextMessageMenu.qml'
--- plugins/Ubuntu/Settings/Menus/TextMessageMenu.qml 2014-11-12 10:49:22 +0000
+++ plugins/Ubuntu/Settings/Menus/TextMessageMenu.qml 2015-04-22 10:48:20 +0000
@@ -32,7 +32,6 @@
32 signal replied(string value)32 signal replied(string value)
3333
34 footer: USC.ActionTextField {34 footer: USC.ActionTextField {
35
36 activateEnabled: menu.replyEnabled35 activateEnabled: menu.replyEnabled
37 buttonText: menu.replyButtonText36 buttonText: menu.replyButtonText
38 textHint: menu.replyHintText37 textHint: menu.replyHintText
3938
=== modified file 'tests/qmltests/Menus/tst_SnapDecisionMenu.qml'
--- tests/qmltests/Menus/tst_SnapDecisionMenu.qml 2014-08-14 11:19:43 +0000
+++ tests/qmltests/Menus/tst_SnapDecisionMenu.qml 2015-04-22 10:48:20 +0000
@@ -95,19 +95,41 @@
95 target: messageMenuSelected95 target: messageMenuSelected
96 }96 }
9797
98 SignalSpy {
99 id: signalSpyTriggered
100 signalName: "triggered"
101 target: messageMenuSelected
102 }
103
98 UbuntuTestCase {104 UbuntuTestCase {
99 name: "SnapDecisionMenu"105 name: "SnapDecisionMenu"
100 when: windowShown106 when: windowShown
101107
102 function init() {108 function cleanup() {
109 textMessageReply = "";
110 messageMenu.replyEnabled = true;
111 messageMenuSelected.selected = false;
112
103 signalSpyIconActivated.clear();113 signalSpyIconActivated.clear();
104 signalSpyDismiss.clear();114 signalSpyDismiss.clear();
105 signalSpyActionActivated.clear();115 signalSpyActionActivated.clear();
106 signalSpyReply.clear();116 signalSpyReply.clear();
107 textMessageReply = "";117 signalSpyTriggered.clear();
108118
109 messageMenu.replyEnabled = true;119 messageMenu.replyExpanded = false;
110 messageMenuSelected.selected = false;120 var replyText = findChild(messageMenu, "replyText");
121 verify(replyText !== undefined, "Reply text not found");
122 replyText.text = "";
123
124 messageMenuRemovable.replyExpanded = false;
125 replyText = findChild(messageMenuRemovable, "replyText");
126 verify(replyText !== undefined, "Reply text not found");
127 replyText.text = "";
128
129 messageMenuSelected.replyExpanded = false;
130 replyText = findChild(messageMenuSelected, "replyText");
131 verify(replyText !== undefined, "Reply text not found");
132 replyText.text = "";
111 }133 }
112134
113 function test_title_data() {135 function test_title_data() {
@@ -294,5 +316,29 @@
294 { tag: 'reply3', index: 2, expected: "reply3" }316 { tag: 'reply3', index: 2, expected: "reply3" }
295 ]317 ]
296 }318 }
319
320 function test_multipleLineReply_lp1396058() {
321 messageMenuSelected.selected = true;
322 messageMenuSelected.replyEnabled = true;
323
324 var replyText = findChild(messageMenuSelected, "replyText");
325 verify(replyText !== undefined, "Reply text not found");
326
327 var messageButton = findChild(messageMenuSelected, "messageButton");
328 verify(messageButton !== undefined, "Message button not found");
329 mouseClick(messageButton, messageButton.width / 2, messageButton.height / 2);
330
331 mouseClick(replyText, replyText.width / 2, replyText.height / 2);
332 compare(replyText.focus, true, "Reply text should have focus after mouse click");
333
334 keyClick("a"); keyClick("b");
335 keyClick(Qt.Key_Return);
336 keyClick("c"); keyClick("d");
337
338 compare(signalSpyTriggered.count, 0, "Item should not have triggered on 'return'")
339
340 compare(replyText.focus, true, "Reply text should still have focus after return pressed");
341 compare(replyText.text, "ab\ncd");
342 }
297 }343 }
298}344}
299345
=== modified file 'tests/qmltests/Menus/tst_TextMessageMenu.qml'
--- tests/qmltests/Menus/tst_TextMessageMenu.qml 2014-08-14 11:19:43 +0000
+++ tests/qmltests/Menus/tst_TextMessageMenu.qml 2015-04-22 10:48:20 +0000
@@ -88,18 +88,37 @@
88 target: messageMenuSelected88 target: messageMenuSelected
89 }89 }
9090
91 SignalSpy {
92 id: signalSpyTriggered
93 signalName: "triggered"
94 target: messageMenuSelected
95 }
96
91 UbuntuTestCase {97 UbuntuTestCase {
92 name: "TextMessageMenu"98 name: "TextMessageMenu"
93 when: windowShown99 when: windowShown
94100
95 function init() {101 function cleanup() {
102 textMessageReply = "";
103 messageMenu.replyEnabled = true;
104 messageMenuSelected.selected = false;
105
96 signalSpyIconActivated.clear();106 signalSpyIconActivated.clear();
97 signalSpyDismiss.clear();107 signalSpyDismiss.clear();
98 signalSpyReply.clear();108 signalSpyReply.clear();
99 textMessageReply = "";109 signalSpyTriggered.clear();
100110
101 messageMenu.replyEnabled = true;111 var replyText = findChild(messageMenu, "replyText");
102 messageMenuSelected.selected = false;112 verify(replyText !== undefined, "Reply text not found");
113 replyText.text = "";
114
115 replyText = findChild(messageMenuRemovable, "replyText");
116 verify(replyText !== undefined, "Reply text not found");
117 replyText.text = "";
118
119 replyText = findChild(messageMenuSelected, "replyText");
120 verify(replyText !== undefined, "Reply text not found");
121 replyText.text = "";
103 }122 }
104123
105 function test_title_data() {124 function test_title_data() {
@@ -248,5 +267,25 @@
248 compare(signalSpyReply.count > 0, true);267 compare(signalSpyReply.count > 0, true);
249 compare(textMessageReply, "reply1", "Text message did not reply with correct text.");268 compare(textMessageReply, "reply1", "Text message did not reply with correct text.");
250 }269 }
270
271 function test_multipleLineReply_lp1396058() {
272 messageMenuSelected.selected = true;
273 messageMenuSelected.replyEnabled = true;
274
275 var replyText = findChild(messageMenuSelected, "replyText");
276 verify(replyText !== undefined, "Reply text not found");
277
278 mouseClick(replyText, replyText.width / 2, replyText.height / 2);
279 compare(replyText.focus, true, "Reply text should have focus after mouse click");
280
281 keyClick("a"); keyClick("b");
282 keyClick(Qt.Key_Return);
283 keyClick("c"); keyClick("d");
284
285 compare(signalSpyTriggered.count, 0, "Item should not have triggered on 'return'")
286
287 compare(replyText.focus, true, "Reply text should still have focus after return pressed");
288 compare(replyText.text, "ab\ncd");
289 }
251 }290 }
252}291}

Subscribers

People subscribed via source and target branches

to all changes: