Skip to content

Commit 2de6926

Browse files
committed
compose_box: Send typing notifications on content changes
We added getTypingNotificationNarrow to reuse the same code for both _StreamContentInput and _FixedDestinationContentInput. This callback function parallels _SendButton.getDestination -- both of them return a computed value depending on the states of the Composecontrollers. It would have been a concern if we intend to use this on the build function, because it essentially carries a mutable state that the Flutter Engine does not necessarily know about. However, simimlar to _SendButton, we only use that on callback functions, not on user visible elements. Plus, _StreamContentInput actually listens to changes to topicController, but the callback doesn't rely on that behavior. Signed-off-by: Zixuan James Li <[email protected]>
1 parent 12598e4 commit 2de6926

File tree

2 files changed

+191
-0
lines changed

2 files changed

+191
-0
lines changed

lib/widgets/compose_box.dart

+60
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.getTypingNotificationNarrow,
275276
required this.controller,
276277
required this.focusNode,
277278
required this.hintText,
278279
});
279280

280281
final Narrow narrow;
282+
final SendableNarrow Function() getTypingNotificationNarrow;
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+
store.typingNotifier.handleTypingStatusUpdate(
328+
// Check and send a "typing stopped" notification eagerly
329+
// if the text gets cleared.
330+
destination: widget.controller.text.isEmpty
331+
? null : widget.getTypingNotificationNarrow());
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.handleTypingStatusUpdate(destination: null);
345+
}
346+
290347
@override
291348
Widget build(BuildContext context) {
292349
ColorScheme colorScheme = Theme.of(context).colorScheme;
@@ -375,6 +432,8 @@ class _StreamContentInputState extends State<_StreamContentInput> {
375432
?? zulipLocalizations.composeBoxUnknownChannelName;
376433
return _ContentInput(
377434
narrow: widget.narrow,
435+
getTypingNotificationNarrow: () => TopicNarrow(
436+
widget.narrow.streamId, widget.topicController.textNormalized),
378437
controller: widget.controller,
379438
focusNode: widget.focusNode,
380439
hintText: zulipLocalizations.composeBoxChannelContentHint(streamName, _topicTextNormalized));
@@ -451,6 +510,7 @@ class _FixedDestinationContentInput extends StatelessWidget {
451510
Widget build(BuildContext context) {
452511
return _ContentInput(
453512
narrow: narrow,
513+
getTypingNotificationNarrow: () => narrow,
454514
controller: controller,
455515
focusNode: focusNode,
456516
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)