Ubuntu Settings Components

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

  1. lp1396058-multi-line-messages
  2. Merge into trunk
Proposed by Nick Dedekind on 2015-04-22
Status: Merged
Approved by: Andrea Cimitan on 2015-06-24
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) 2015-04-22 Approve on 2015-06-24
PS Jenkins bot (community) continuous-integration Approve on 2015-04-22
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.
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
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
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.

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
1=== modified file 'examples/MessageComponents.qml'
2--- examples/MessageComponents.qml 2014-09-11 13:30:09 +0000
3+++ examples/MessageComponents.qml 2015-04-22 10:48:20 +0000
4@@ -92,6 +92,7 @@
5 body: model.body
6 time: model.time
7 removable: true
8+ replyHintText: "Reply"
9
10 onTriggered: {
11 selected = !selected;
12@@ -108,6 +109,7 @@
13 body: model.body
14 time: model.time
15 removable: true
16+ replyHintText: "Reply"
17
18 onTriggered: {
19 selected = !selected;
20
21=== modified file 'plugins/Ubuntu/Settings/Components/ActionTextField.qml'
22--- plugins/Ubuntu/Settings/Components/ActionTextField.qml 2014-11-12 10:49:22 +0000
23+++ plugins/Ubuntu/Settings/Components/ActionTextField.qml 2015-04-22 10:48:20 +0000
24@@ -33,7 +33,7 @@
25
26 implicitHeight: layout.implicitHeight
27
28- RowLayout {
29+ Row {
30 id: layout
31 anchors {
32 left: parent.left
33@@ -41,13 +41,11 @@
34 }
35 spacing: units.gu(1)
36
37- TextField {
38+ TextArea {
39 id: replyField
40 objectName: "replyText"
41-
42- hasClearButton: false
43-
44- Layout.fillWidth: true
45+ autoSize: true
46+ width: parent.width - layout.spacing - sendButton.width
47
48 onEnabledChanged: {
49 //Make sure that the component lost focus when enabled = false,
50
51=== modified file 'plugins/Ubuntu/Settings/Menus/SnapDecisionMenu.qml'
52--- plugins/Ubuntu/Settings/Menus/SnapDecisionMenu.qml 2014-08-14 11:19:43 +0000
53+++ plugins/Ubuntu/Settings/Menus/SnapDecisionMenu.qml 2015-04-22 10:48:20 +0000
54@@ -31,6 +31,8 @@
55
56 property bool replyEnabled: true
57 property string replyButtonText: i18n.tr("Send")
58+ property string replyHintText
59+ property bool replyExpanded: false
60
61 signal actionActivated
62 signal replied(string value)
63@@ -57,11 +59,7 @@
64 Layout.fillWidth: true
65
66 onClicked: {
67- if (reply.state === "") {
68- reply.state = "expanded";
69- } else {
70- reply.state = "";
71- }
72+ menu.replyExpanded = !menu.replyExpanded;
73 }
74 }
75
76@@ -84,10 +82,11 @@
77
78 Layout.fillWidth: true
79 Layout.fillHeight: true
80- visible: state == "expanded"
81+ visible: menu.replyExpanded
82
83 activateEnabled: menu.replyEnabled
84 buttonText: menu.replyButtonText
85+ textHint: menu.replyHintText
86
87 onActivated: {
88 menu.replied(value);
89
90=== modified file 'plugins/Ubuntu/Settings/Menus/TextMessageMenu.qml'
91--- plugins/Ubuntu/Settings/Menus/TextMessageMenu.qml 2014-11-12 10:49:22 +0000
92+++ plugins/Ubuntu/Settings/Menus/TextMessageMenu.qml 2015-04-22 10:48:20 +0000
93@@ -32,7 +32,6 @@
94 signal replied(string value)
95
96 footer: USC.ActionTextField {
97-
98 activateEnabled: menu.replyEnabled
99 buttonText: menu.replyButtonText
100 textHint: menu.replyHintText
101
102=== modified file 'tests/qmltests/Menus/tst_SnapDecisionMenu.qml'
103--- tests/qmltests/Menus/tst_SnapDecisionMenu.qml 2014-08-14 11:19:43 +0000
104+++ tests/qmltests/Menus/tst_SnapDecisionMenu.qml 2015-04-22 10:48:20 +0000
105@@ -95,19 +95,41 @@
106 target: messageMenuSelected
107 }
108
109+ SignalSpy {
110+ id: signalSpyTriggered
111+ signalName: "triggered"
112+ target: messageMenuSelected
113+ }
114+
115 UbuntuTestCase {
116 name: "SnapDecisionMenu"
117 when: windowShown
118
119- function init() {
120+ function cleanup() {
121+ textMessageReply = "";
122+ messageMenu.replyEnabled = true;
123+ messageMenuSelected.selected = false;
124+
125 signalSpyIconActivated.clear();
126 signalSpyDismiss.clear();
127 signalSpyActionActivated.clear();
128 signalSpyReply.clear();
129- textMessageReply = "";
130-
131- messageMenu.replyEnabled = true;
132- messageMenuSelected.selected = false;
133+ signalSpyTriggered.clear();
134+
135+ messageMenu.replyExpanded = false;
136+ var replyText = findChild(messageMenu, "replyText");
137+ verify(replyText !== undefined, "Reply text not found");
138+ replyText.text = "";
139+
140+ messageMenuRemovable.replyExpanded = false;
141+ replyText = findChild(messageMenuRemovable, "replyText");
142+ verify(replyText !== undefined, "Reply text not found");
143+ replyText.text = "";
144+
145+ messageMenuSelected.replyExpanded = false;
146+ replyText = findChild(messageMenuSelected, "replyText");
147+ verify(replyText !== undefined, "Reply text not found");
148+ replyText.text = "";
149 }
150
151 function test_title_data() {
152@@ -294,5 +316,29 @@
153 { tag: 'reply3', index: 2, expected: "reply3" }
154 ]
155 }
156+
157+ function test_multipleLineReply_lp1396058() {
158+ messageMenuSelected.selected = true;
159+ messageMenuSelected.replyEnabled = true;
160+
161+ var replyText = findChild(messageMenuSelected, "replyText");
162+ verify(replyText !== undefined, "Reply text not found");
163+
164+ var messageButton = findChild(messageMenuSelected, "messageButton");
165+ verify(messageButton !== undefined, "Message button not found");
166+ mouseClick(messageButton, messageButton.width / 2, messageButton.height / 2);
167+
168+ mouseClick(replyText, replyText.width / 2, replyText.height / 2);
169+ compare(replyText.focus, true, "Reply text should have focus after mouse click");
170+
171+ keyClick("a"); keyClick("b");
172+ keyClick(Qt.Key_Return);
173+ keyClick("c"); keyClick("d");
174+
175+ compare(signalSpyTriggered.count, 0, "Item should not have triggered on 'return'")
176+
177+ compare(replyText.focus, true, "Reply text should still have focus after return pressed");
178+ compare(replyText.text, "ab\ncd");
179+ }
180 }
181 }
182
183=== modified file 'tests/qmltests/Menus/tst_TextMessageMenu.qml'
184--- tests/qmltests/Menus/tst_TextMessageMenu.qml 2014-08-14 11:19:43 +0000
185+++ tests/qmltests/Menus/tst_TextMessageMenu.qml 2015-04-22 10:48:20 +0000
186@@ -88,18 +88,37 @@
187 target: messageMenuSelected
188 }
189
190+ SignalSpy {
191+ id: signalSpyTriggered
192+ signalName: "triggered"
193+ target: messageMenuSelected
194+ }
195+
196 UbuntuTestCase {
197 name: "TextMessageMenu"
198 when: windowShown
199
200- function init() {
201+ function cleanup() {
202+ textMessageReply = "";
203+ messageMenu.replyEnabled = true;
204+ messageMenuSelected.selected = false;
205+
206 signalSpyIconActivated.clear();
207 signalSpyDismiss.clear();
208 signalSpyReply.clear();
209- textMessageReply = "";
210-
211- messageMenu.replyEnabled = true;
212- messageMenuSelected.selected = false;
213+ signalSpyTriggered.clear();
214+
215+ var replyText = findChild(messageMenu, "replyText");
216+ verify(replyText !== undefined, "Reply text not found");
217+ replyText.text = "";
218+
219+ replyText = findChild(messageMenuRemovable, "replyText");
220+ verify(replyText !== undefined, "Reply text not found");
221+ replyText.text = "";
222+
223+ replyText = findChild(messageMenuSelected, "replyText");
224+ verify(replyText !== undefined, "Reply text not found");
225+ replyText.text = "";
226 }
227
228 function test_title_data() {
229@@ -248,5 +267,25 @@
230 compare(signalSpyReply.count > 0, true);
231 compare(textMessageReply, "reply1", "Text message did not reply with correct text.");
232 }
233+
234+ function test_multipleLineReply_lp1396058() {
235+ messageMenuSelected.selected = true;
236+ messageMenuSelected.replyEnabled = true;
237+
238+ var replyText = findChild(messageMenuSelected, "replyText");
239+ verify(replyText !== undefined, "Reply text not found");
240+
241+ mouseClick(replyText, replyText.width / 2, replyText.height / 2);
242+ compare(replyText.focus, true, "Reply text should have focus after mouse click");
243+
244+ keyClick("a"); keyClick("b");
245+ keyClick(Qt.Key_Return);
246+ keyClick("c"); keyClick("d");
247+
248+ compare(signalSpyTriggered.count, 0, "Item should not have triggered on 'return'")
249+
250+ compare(replyText.focus, true, "Reply text should still have focus after return pressed");
251+ compare(replyText.text, "ab\ncd");
252+ }
253 }
254 }

Subscribers

People subscribed via source and target branches

to all changes: