Skip to content

Commit 559981d

Browse files
committed
compose_box: Send typing notifications on content changes
For a compose box with topic input, the field `destination` contains a mutable state computed from the topic text, so it is crucial that such a compose box handles rebuilds when the topic text is updated. We already did that to keep hintText up-to-date, so it's not a concern. Signed-off-by: Zixuan James Li <[email protected]>
1 parent 30a2ec7 commit 559981d

File tree

2 files changed

+192
-0
lines changed

2 files changed

+192
-0
lines changed

lib/widgets/compose_box.dart

+61
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,61 @@ class _ContentInput extends StatefulWidget {
287289
}
288290

289291
class _ContentInputState extends State<_ContentInput> {
292+
String? _prevText;
293+
294+
@override
295+
void initState() {
296+
super.initState();
297+
widget.controller.addListener(_contentChanged);
298+
widget.focusNode.addListener(_focusChanged);
299+
}
300+
301+
@override
302+
void didUpdateWidget(covariant _ContentInput oldWidget) {
303+
super.didUpdateWidget(oldWidget);
304+
if (widget.controller != oldWidget.controller) {
305+
oldWidget.controller.removeListener(_contentChanged);
306+
oldWidget.focusNode.removeListener(_focusChanged);
307+
widget.controller.addListener(_contentChanged);
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+
if (_prevText == widget.controller.text) {
321+
// [TextEditingController] also notifies the listeners
322+
// on selection updates. Return early because we do not
323+
// consider that as typing activity.
324+
return;
325+
}
326+
final store = PerAccountStoreWidget.of(context);
327+
if (widget.controller.text.isEmpty) {
328+
store.typingNotifier.stoppedComposing();
329+
} else {
330+
store.typingNotifier.keystroke(widget.destination);
331+
}
332+
_prevText = widget.controller.text;
333+
}
334+
335+
void _focusChanged() {
336+
if (widget.focusNode.hasFocus) {
337+
// Content input getting focus doesn't necessarily mean that
338+
// the user started typing, so do nothing.
339+
return;
340+
}
341+
// Losing focus usually indicates that the user has navigated away
342+
// or clicked on other UI elements.
343+
final store = PerAccountStoreWidget.of(context);
344+
store.typingNotifier.stoppedComposing();
345+
}
346+
290347
@override
291348
Widget build(BuildContext context) {
292349
ColorScheme colorScheme = Theme.of(context).colorScheme;
@@ -337,6 +394,8 @@ class _StreamContentInput extends StatefulWidget {
337394
}
338395

339396
class _StreamContentInputState extends State<_StreamContentInput> {
397+
SendableNarrow get _destination => TopicNarrow(
398+
widget.narrow.streamId, widget.topicController.textNormalized);
340399
late String _topicTextNormalized;
341400

342401
void _topicChanged() {
@@ -375,6 +434,7 @@ class _StreamContentInputState extends State<_StreamContentInput> {
375434
?? zulipLocalizations.composeBoxUnknownChannelName;
376435
return _ContentInput(
377436
narrow: widget.narrow,
437+
destination: _destination,
378438
controller: widget.controller,
379439
focusNode: widget.focusNode,
380440
hintText: zulipLocalizations.composeBoxChannelContentHint(streamName, _topicTextNormalized));
@@ -451,6 +511,7 @@ class _FixedDestinationContentInput extends StatelessWidget {
451511
Widget build(BuildContext context) {
452512
return _ContentInput(
453513
narrow: narrow,
514+
destination: narrow,
454515
controller: controller,
455516
focusNode: focusNode,
456517
hintText: _hintText(context));

test/widgets/compose_box_test.dart

+131
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,16 @@ import 'package:zulip/model/localizations.dart';
1515
import 'package:zulip/model/narrow.dart';
1616
import 'package:zulip/model/store.dart';
1717
import 'package:zulip/model/typing_status.dart';
18+
import 'package:zulip/widgets/app.dart';
1819
import 'package:zulip/widgets/compose_box.dart';
20+
import 'package:zulip/widgets/page.dart';
1921

2022
import '../api/fake_api.dart';
2123
import '../example_data.dart' as eg;
2224
import '../flutter_checks.dart';
2325
import '../model/binding.dart';
2426
import '../model/test_store.dart';
27+
import '../model/typing_status_test.dart';
2528
import '../stdlib_checks.dart';
2629
import 'dialog_checks.dart';
2730
import 'test_app.dart';
@@ -201,6 +204,134 @@ void main() {
201204
});
202205
});
203206

207+
group('ComposeBox typing notification', () {
208+
const narrow = TopicNarrow(123, 'some topic');
209+
210+
// This uses a high feature level to test with the latest version of the
211+
// setTypingNotifier API.
212+
final account = eg.account(
213+
user: eg.selfUser, zulipFeatureLevel: eg.futureZulipFeatureLevel);
214+
215+
final contentInputFinder = find.byWidgetPredicate(
216+
(widget) => widget is TextField && widget.controller is ComposeContentController);
217+
final topicInputFinder = find.byWidgetPredicate(
218+
(widget) => widget is TextField && widget.controller is ComposeTopicController);
219+
220+
void checkTypingRequest(TypingOp op, SendableNarrow narrow) =>
221+
checkSetTypingStatusRequests(connection, [(op, narrow)]);
222+
223+
Future<void> checkStartTyping(WidgetTester tester, SendableNarrow narrow) async {
224+
connection.prepare(json: {});
225+
await tester.enterText(contentInputFinder, 'hello world');
226+
checkTypingRequest(TypingOp.start, narrow);
227+
}
228+
229+
testWidgets('smoke TopicNarrow', (tester) async {
230+
await prepareComposeBox(tester, narrow: narrow, account: account);
231+
232+
await checkStartTyping(tester, narrow);
233+
234+
connection.prepare(json: {});
235+
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
236+
checkTypingRequest(TypingOp.stop, narrow);
237+
});
238+
239+
testWidgets('smoke DmNarrow', (tester) async {
240+
final narrow = DmNarrow.withUsers(
241+
[eg.otherUser.userId], selfUserId: eg.selfUser.userId);
242+
await prepareComposeBox(tester, narrow: narrow, account: account);
243+
244+
await checkStartTyping(tester, narrow);
245+
246+
connection.prepare(json: {});
247+
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
248+
checkTypingRequest(TypingOp.stop, narrow);
249+
});
250+
251+
testWidgets('smoke ChannelNarrow', (tester) async {
252+
const narrow = ChannelNarrow(123);
253+
final destinationNarrow = TopicNarrow(narrow.streamId, 'test topic');
254+
await prepareComposeBox(tester, narrow: narrow, account: account);
255+
256+
await tester.enterText(topicInputFinder, destinationNarrow.topic);
257+
// Remove an irrelevant topic request.
258+
check(connection.takeRequests()).single
259+
..method.equals('GET')
260+
..url.path.equals('/api/v1/users/me/123/topics');
261+
262+
await checkStartTyping(tester, destinationNarrow);
263+
264+
connection.prepare(json: {});
265+
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
266+
checkTypingRequest(TypingOp.stop, destinationNarrow);
267+
});
268+
269+
testWidgets('clearing text stops typing notification', (tester) async {
270+
await prepareComposeBox(tester, narrow: narrow, account: account);
271+
272+
await checkStartTyping(tester, narrow);
273+
274+
connection.prepare(json: {});
275+
await tester.enterText(contentInputFinder, '');
276+
checkTypingRequest(TypingOp.stop, narrow);
277+
});
278+
279+
Future<void> prepareComposeBoxWithNavigation(WidgetTester tester) async {
280+
addTearDown(testBinding.reset);
281+
await testBinding.globalStore.add(account, eg.initialSnapshot(
282+
zulipFeatureLevel: account.zulipFeatureLevel));
283+
284+
await tester.pumpWidget(const ZulipApp());
285+
await tester.pump();
286+
final navigator = await ZulipApp.navigator;
287+
navigator.push(MaterialAccountWidgetRoute(
288+
accountId: account.id, page: const ComposeBox(narrow: narrow)));
289+
await tester.pumpAndSettle();
290+
291+
store = await testBinding.globalStore.perAccount(account.id);
292+
connection = store.connection as FakeApiConnection;
293+
}
294+
295+
testWidgets('navigating away stops typing notification', (tester) async {
296+
await prepareComposeBoxWithNavigation(tester);
297+
298+
await checkStartTyping(tester, narrow);
299+
300+
connection.prepare(json: {});
301+
(await ZulipApp.navigator).pop();
302+
await tester.pump(Duration.zero);
303+
checkTypingRequest(TypingOp.stop, narrow);
304+
});
305+
306+
testWidgets('for content input, unfocusing stops typing notification and refocusing does not start it', (tester) async {
307+
const narrow = ChannelNarrow(123);
308+
final destinationNarrow = TopicNarrow(narrow.streamId, 'test topic');
309+
await prepareComposeBox(tester, narrow: narrow, account: account);
310+
311+
await tester.enterText(topicInputFinder, destinationNarrow.topic);
312+
// Remove an irrelevant topic request.
313+
check(connection.takeRequests()).single
314+
..method.equals('GET')
315+
..url.path.equals('/api/v1/users/me/123/topics');
316+
317+
connection.prepare(json: {});
318+
await tester.enterText(contentInputFinder, 'hello world');
319+
checkTypingRequest(TypingOp.start, destinationNarrow);
320+
321+
connection.prepare(json: {});
322+
// Move focus to the topic input
323+
await tester.tap(topicInputFinder);
324+
await tester.pump(Duration.zero);
325+
checkTypingRequest(TypingOp.stop, destinationNarrow);
326+
327+
await tester.tap(contentInputFinder);
328+
await tester.pump(Duration.zero);
329+
// Refocusing on the content input would trigger an update to
330+
// [TextEditingController.selection]. Still, we expect no
331+
// "typing started" request if the text remains the same.
332+
});
333+
});
334+
204335
group('message-send request response', () {
205336
Future<void> setupAndTapSend(WidgetTester tester, {
206337
required void Function(int messageId) prepareResponse,

0 commit comments

Comments
 (0)