Skip to content

Commit f7533ad

Browse files
committed
compose_box: Send typing notices on content/focus changes
For a compose box with topic input, the field `destination` contains a mutable state computed from the topic text. This implementation is similar to hintText's to still keep everything up-to-date. When testing typing activities that do not end with a "typing stopped" notice, we need to wait for the idle timer to expire so that the test does not end with pending timers. Signed-off-by: Zixuan James Li <[email protected]>
1 parent da38b63 commit f7533ad

File tree

3 files changed

+231
-27
lines changed

3 files changed

+231
-27
lines changed

lib/widgets/compose_box.dart

+57
Original file line numberDiff line numberDiff line change
@@ -272,12 +272,14 @@ class ComposeContentController extends ComposeController<ContentValidationError>
272272
class _ContentInput extends StatefulWidget {
273273
const _ContentInput({
274274
required this.narrow,
275+
required this.destination,
275276
required this.controller,
276277
required this.focusNode,
277278
required this.hintText,
278279
});
279280

280281
final Narrow narrow;
282+
final SendableNarrow destination;
281283
final ComposeContentController controller;
282284
final FocusNode focusNode;
283285
final String hintText;
@@ -287,6 +289,54 @@ class _ContentInput extends StatefulWidget {
287289
}
288290

289291
class _ContentInputState extends State<_ContentInput> {
292+
@override
293+
void initState() {
294+
super.initState();
295+
widget.controller.addListener(_contentChanged);
296+
widget.focusNode.addListener(_focusChanged);
297+
}
298+
299+
@override
300+
void didUpdateWidget(covariant _ContentInput oldWidget) {
301+
super.didUpdateWidget(oldWidget);
302+
if (widget.controller != oldWidget.controller) {
303+
oldWidget.controller.removeListener(_contentChanged);
304+
widget.controller.addListener(_contentChanged);
305+
}
306+
if (widget.focusNode != oldWidget.focusNode) {
307+
oldWidget.focusNode.removeListener(_focusChanged);
308+
widget.focusNode.addListener(_focusChanged);
309+
}
310+
}
311+
312+
@override
313+
void dispose() {
314+
widget.controller.removeListener(_contentChanged);
315+
widget.focusNode.removeListener(_focusChanged);
316+
super.dispose();
317+
}
318+
319+
void _contentChanged() {
320+
final store = PerAccountStoreWidget.of(context);
321+
(widget.controller.text.isEmpty)
322+
? store.typingNotifier.stoppedComposing()
323+
: store.typingNotifier.keystroke(widget.destination);
324+
}
325+
326+
void _focusChanged() {
327+
if (widget.focusNode.hasFocus) {
328+
// Content input getting focus doesn't necessarily mean that
329+
// the user started typing, so do nothing.
330+
return;
331+
}
332+
// Losing focus usually indicates that the user has navigated away or
333+
// clicked on other UI elements. On Android, this does not get triggered
334+
// unless the user navigates away from the page or focus on another input.
335+
// See: https://github.com/zulip/zulip-flutter/pull/897#discussion_r1818188643
336+
final store = PerAccountStoreWidget.of(context);
337+
store.typingNotifier.stoppedComposing();
338+
}
339+
290340
@override
291341
Widget build(BuildContext context) {
292342
ColorScheme colorScheme = Theme.of(context).colorScheme;
@@ -375,6 +425,8 @@ class _StreamContentInputState extends State<_StreamContentInput> {
375425
?? zulipLocalizations.composeBoxUnknownChannelName;
376426
return _ContentInput(
377427
narrow: widget.narrow,
428+
destination: TopicNarrow(
429+
widget.narrow.streamId, widget.topicController.textNormalized),
378430
controller: widget.controller,
379431
focusNode: widget.focusNode,
380432
hintText: zulipLocalizations.composeBoxChannelContentHint(streamName, _topicTextNormalized));
@@ -451,6 +503,7 @@ class _FixedDestinationContentInput extends StatelessWidget {
451503
Widget build(BuildContext context) {
452504
return _ContentInput(
453505
narrow: narrow,
506+
destination: narrow,
454507
controller: controller,
455508
focusNode: focusNode,
456509
hintText: _hintText(context));
@@ -823,6 +876,10 @@ class _SendButtonState extends State<_SendButton> {
823876
final content = widget.contentController.textNormalized;
824877

825878
widget.contentController.clear();
879+
// The following `stoppedComposing` call is currently redundant,
880+
// because clearing input sends a "typing stopped" notice.
881+
// It will be necessary once we resolve #720.
882+
store.typingNotifier.stoppedComposing();
826883

827884
try {
828885
// TODO(#720) clear content input only on success response;

test/model/typing_status_test.dart

+27-27
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,33 @@ import '../stdlib_checks.dart';
1717
import 'binding.dart';
1818
import 'test_store.dart';
1919

20+
void checkSetTypingStatusRequests(
21+
List<http.BaseRequest> requests,
22+
List<(TypingOp, SendableNarrow)> expected,
23+
) {
24+
Condition<Object?> conditionTypingRequest(Map<String, String> expected) {
25+
return (Subject<Object?> it) => it.isA<http.Request>()
26+
..method.equals('POST')
27+
..url.path.equals('/api/v1/typing')
28+
..bodyFields.deepEquals(expected);
29+
}
30+
31+
check(requests).deepEquals([
32+
for (final (op, narrow) in expected)
33+
switch (narrow) {
34+
TopicNarrow() => conditionTypingRequest({
35+
'type': 'channel',
36+
'op': op.toJson(),
37+
'stream_id': narrow.streamId.toString(),
38+
'topic': narrow.topic}),
39+
DmNarrow() => conditionTypingRequest({
40+
'type': 'direct',
41+
'op': op.toJson(),
42+
'to': jsonEncode(narrow.allRecipientIds)}),
43+
}
44+
]);
45+
}
46+
2047
void main() {
2148
TestZulipBinding.ensureInitialized();
2249

@@ -228,33 +255,6 @@ void main() {
228255
late FakeApiConnection connection;
229256
late TopicNarrow narrow;
230257

231-
void checkSetTypingStatusRequests(
232-
List<http.BaseRequest> requests,
233-
List<(TypingOp, SendableNarrow)> expected,
234-
) {
235-
Condition<Object?> conditionTypingRequest(Map<String, String> expected) {
236-
return (Subject<Object?> it) => it.isA<http.Request>()
237-
..method.equals('POST')
238-
..url.path.equals('/api/v1/typing')
239-
..bodyFields.deepEquals(expected);
240-
}
241-
242-
check(requests).deepEquals([
243-
for (final (op, narrow) in expected)
244-
switch (narrow) {
245-
TopicNarrow() => conditionTypingRequest({
246-
'type': 'channel',
247-
'op': op.toJson(),
248-
'stream_id': narrow.streamId.toString(),
249-
'topic': narrow.topic}),
250-
DmNarrow() => conditionTypingRequest({
251-
'type': 'direct',
252-
'op': op.toJson(),
253-
'to': jsonEncode(narrow.allRecipientIds)}),
254-
}
255-
]);
256-
}
257-
258258
void checkTypingRequest(TypingOp op, SendableNarrow narrow) =>
259259
checkSetTypingStatusRequests(connection.takeRequests(), [(op, narrow)]);
260260

test/widgets/compose_box_test.dart

+147
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import 'dart:async';
12
import 'dart:convert';
23

34
import 'package:checks/checks.dart';
@@ -15,13 +16,16 @@ import 'package:zulip/model/localizations.dart';
1516
import 'package:zulip/model/narrow.dart';
1617
import 'package:zulip/model/store.dart';
1718
import 'package:zulip/model/typing_status.dart';
19+
import 'package:zulip/widgets/app.dart';
1820
import 'package:zulip/widgets/compose_box.dart';
21+
import 'package:zulip/widgets/page.dart';
1922

2023
import '../api/fake_api.dart';
2124
import '../example_data.dart' as eg;
2225
import '../flutter_checks.dart';
2326
import '../model/binding.dart';
2427
import '../model/test_store.dart';
28+
import '../model/typing_status_test.dart';
2529
import '../stdlib_checks.dart';
2630
import 'dialog_checks.dart';
2731
import 'test_app.dart';
@@ -216,6 +220,149 @@ void main() {
216220
});
217221
});
218222

223+
group('ComposeBox typing notices', () {
224+
const narrow = TopicNarrow(123, 'some topic');
225+
226+
void checkTypingRequest(TypingOp op, SendableNarrow narrow) =>
227+
checkSetTypingStatusRequests(connection.takeRequests(), [(op, narrow)]);
228+
229+
Future<void> checkStartTyping(WidgetTester tester, SendableNarrow narrow) async {
230+
connection.prepare(json: {});
231+
await tester.enterText(contentInputFinder, 'hello world');
232+
checkTypingRequest(TypingOp.start, narrow);
233+
}
234+
235+
testWidgets('smoke TopicNarrow', (tester) async {
236+
await prepareComposeBox(tester, narrow: narrow);
237+
238+
await checkStartTyping(tester, narrow);
239+
240+
connection.prepare(json: {});
241+
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
242+
checkTypingRequest(TypingOp.stop, narrow);
243+
});
244+
245+
testWidgets('smoke DmNarrow', (tester) async {
246+
final narrow = DmNarrow.withUsers(
247+
[eg.otherUser.userId], selfUserId: eg.selfUser.userId);
248+
await prepareComposeBox(tester, narrow: narrow);
249+
250+
await checkStartTyping(tester, narrow);
251+
252+
connection.prepare(json: {});
253+
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
254+
checkTypingRequest(TypingOp.stop, narrow);
255+
});
256+
257+
testWidgets('smoke ChannelNarrow', (tester) async {
258+
const narrow = ChannelNarrow(123);
259+
final destinationNarrow = TopicNarrow(narrow.streamId, 'test topic');
260+
await prepareComposeBox(
261+
tester, narrow: narrow, topic: destinationNarrow.topic);
262+
263+
await checkStartTyping(tester, destinationNarrow);
264+
265+
connection.prepare(json: {});
266+
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
267+
checkTypingRequest(TypingOp.stop, destinationNarrow);
268+
});
269+
270+
testWidgets('clearing text sends a "typing stopped" notice', (tester) async {
271+
await prepareComposeBox(tester, narrow: narrow);
272+
273+
await checkStartTyping(tester, narrow);
274+
275+
connection.prepare(json: {});
276+
await tester.enterText(contentInputFinder, '');
277+
checkTypingRequest(TypingOp.stop, narrow);
278+
});
279+
280+
testWidgets('hitting send button sends a "typing stopped" notice', (tester) async {
281+
await prepareComposeBox(tester, narrow: narrow);
282+
283+
await checkStartTyping(tester, narrow);
284+
285+
connection.prepare(json: {});
286+
connection.prepare(json: SendMessageResult(id: 123).toJson());
287+
await tester.tap(find.byIcon(Icons.send));
288+
await tester.pump(Duration.zero);
289+
final requests = connection.takeRequests();
290+
checkSetTypingStatusRequests([requests.first], [(TypingOp.stop, narrow)]);
291+
check(requests).length.equals(2);
292+
});
293+
294+
Future<void> prepareComposeBoxWithNavigation(WidgetTester tester) async {
295+
addTearDown(testBinding.reset);
296+
await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
297+
298+
store = await testBinding.globalStore.perAccount(eg.selfAccount.id);
299+
connection = store.connection as FakeApiConnection;
300+
301+
await tester.pumpWidget(const ZulipApp());
302+
await tester.pump();
303+
final navigator = await ZulipApp.navigator;
304+
unawaited(navigator.push(MaterialAccountWidgetRoute(
305+
accountId: eg.selfAccount.id, page: const ComposeBox(narrow: narrow))));
306+
await tester.pumpAndSettle();
307+
}
308+
309+
testWidgets('navigating away sends a "typing stopped" notice', (tester) async {
310+
await prepareComposeBoxWithNavigation(tester);
311+
312+
await checkStartTyping(tester, narrow);
313+
314+
connection.prepare(json: {});
315+
(await ZulipApp.navigator).pop();
316+
await tester.pump(Duration.zero);
317+
checkTypingRequest(TypingOp.stop, narrow);
318+
});
319+
320+
testWidgets('for content input, unfocusing sends a "typing stopped" notice '
321+
'and refocusing sends a "typing started" notice', (tester) async {
322+
const narrow = ChannelNarrow(123);
323+
final destinationNarrow = TopicNarrow(narrow.streamId, 'test topic');
324+
await prepareComposeBox(
325+
tester, narrow: narrow, topic: destinationNarrow.topic);
326+
327+
await checkStartTyping(tester, destinationNarrow);
328+
329+
connection.prepare(json: {});
330+
FocusManager.instance.primaryFocus!.unfocus();
331+
await tester.pump(Duration.zero);
332+
checkTypingRequest(TypingOp.stop, destinationNarrow);
333+
334+
connection.prepare(json: {});
335+
await tester.tap(contentInputFinder);
336+
checkTypingRequest(TypingOp.start, destinationNarrow);
337+
338+
// Ensures that a "typing stopped" notice is sent when the test ends.
339+
connection.prepare(json: {});
340+
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
341+
checkTypingRequest(TypingOp.stop, destinationNarrow);
342+
});
343+
344+
testWidgets('selection change sends a "typing started" notice', (tester) async {
345+
final controllerKey = await prepareComposeBox(tester, narrow: narrow);
346+
final composeBoxController = controllerKey.currentState!;
347+
348+
await checkStartTyping(tester, narrow);
349+
350+
connection.prepare(json: {});
351+
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
352+
checkTypingRequest(TypingOp.stop, narrow);
353+
354+
connection.prepare(json: {});
355+
composeBoxController.contentController.selection =
356+
const TextSelection(baseOffset: 0, extentOffset: 2);
357+
checkTypingRequest(TypingOp.start, narrow);
358+
359+
// Ensures that a "typing stopped" notice is sent when the test ends.
360+
connection.prepare(json: {});
361+
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
362+
checkTypingRequest(TypingOp.stop, narrow);
363+
});
364+
});
365+
219366
group('message-send request response', () {
220367
Future<void> setupAndTapSend(WidgetTester tester, {
221368
required void Function(int messageId) prepareResponse,

0 commit comments

Comments
 (0)