Skip to content

Commit

Permalink
compose_box: Send typing notifications on content changes
Browse files Browse the repository at this point in the history 
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.

Fixes: zulip#666

Signed-off-by: Zixuan James Li <[email protected]>
  • Loading branch information
@PIG208
PIG208 committed Oct 11, 2024
1 parent e2980f2 commit e97ffdc
Show file tree
Hide file tree
Showing 2 changed files with 196 additions and 0 deletions.
65 changes: 65 additions & 0 deletions lib/widgets/compose_box.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -272,12 +272,14 @@ class ComposeContentController extends ComposeController<ContentValidationError>
class _ContentInput extends StatefulWidget {
const _ContentInput({
required this.narrow,
required this.destination,
required this.controller,
required this.focusNode,
required this.hintText,
});

final Narrow narrow;
final SendableNarrow destination;
final ComposeContentController controller;
final FocusNode focusNode;
final String hintText;
Expand All @@ -287,6 +289,61 @@ class _ContentInput extends StatefulWidget {
}

class _ContentInputState extends State<_ContentInput> {
String? _prevText;

@override
void initState() {
super.initState();
widget.controller.addListener(_contentChanged);
widget.focusNode.addListener(_focusChanged);
}

@override
void didUpdateWidget(covariant _ContentInput oldWidget) {
super.didUpdateWidget(oldWidget);
if (widget.controller != oldWidget.controller) {
oldWidget.controller.removeListener(_contentChanged);
oldWidget.focusNode.removeListener(_focusChanged);
widget.controller.addListener(_contentChanged);
widget.focusNode.addListener(_focusChanged);
}
}

@override
void dispose() {
widget.controller.removeListener(_contentChanged);
widget.focusNode.removeListener(_focusChanged);
super.dispose();
}

void _contentChanged() {
if (_prevText == widget.controller.text) {
// [TextEditingController] also notifies the listeners
// on selection updates. Return early because we do not
// consider that as typing activity.
return;
}
final store = PerAccountStoreWidget.of(context);
if (widget.controller.text.isEmpty) {
store.typingNotifier.stoppedComposing();
} else {
store.typingNotifier.keystroke(widget.destination);
}
_prevText = widget.controller.text;
}

void _focusChanged() {
if (widget.focusNode.hasFocus) {
// Content input getting focus doesn't necessarily mean that
// the user started typing, so do nothing.
return;
}
// Losing focus usually indicates that the user has navigated away
// or clicked on other UI elements.
final store = PerAccountStoreWidget.of(context);
store.typingNotifier.stoppedComposing();
}

@override
Widget build(BuildContext context) {
ColorScheme colorScheme = Theme.of(context).colorScheme;
Expand Down Expand Up @@ -337,6 +394,8 @@ class _StreamContentInput extends StatefulWidget {
}

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

void _topicChanged() {
Expand Down Expand Up @@ -375,6 +434,7 @@ class _StreamContentInputState extends State<_StreamContentInput> {
?? zulipLocalizations.composeBoxUnknownChannelName;
return _ContentInput(
narrow: widget.narrow,
destination: _destination,
controller: widget.controller,
focusNode: widget.focusNode,
hintText: zulipLocalizations.composeBoxChannelContentHint(streamName, _topicTextNormalized));
Expand Down Expand Up @@ -451,6 +511,7 @@ class _FixedDestinationContentInput extends StatelessWidget {
Widget build(BuildContext context) {
return _ContentInput(
narrow: narrow,
destination: narrow,
controller: controller,
focusNode: focusNode,
hintText: _hintText(context));
Expand Down Expand Up @@ -828,6 +889,10 @@ class _SendButtonState extends State<_SendButton> {
// TODO(#720) clear content input only on success response;
// while waiting, put input(s) and send button into a disabled
// "working on it" state (letting input text be selected for copying).
// The following `stoppedComposing` call will cease being redundant
// once we support that, because clearing input sends a "typing stopped"
// notice.
store.typingNotifier.stoppedComposing();
await store.sendMessage(destination: widget.getDestination(), content: content);
} on ApiRequestException catch (e) {
if (!mounted) return;
Expand Down
131 changes: 131 additions & 0 deletions test/widgets/compose_box_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@ import 'package:zulip/model/localizations.dart';
import 'package:zulip/model/narrow.dart';
import 'package:zulip/model/store.dart';
import 'package:zulip/model/typing_status.dart';
import 'package:zulip/widgets/app.dart';
import 'package:zulip/widgets/compose_box.dart';
import 'package:zulip/widgets/page.dart';

import '../api/fake_api.dart';
import '../example_data.dart' as eg;
import '../flutter_checks.dart';
import '../model/binding.dart';
import '../model/test_store.dart';
import '../model/typing_status_test.dart';
import '../stdlib_checks.dart';
import 'dialog_checks.dart';
import 'test_app.dart';
Expand Down Expand Up @@ -201,6 +204,134 @@ void main() {
});
});

group('ComposeBox typing notification', () {
const narrow = TopicNarrow(123, 'some topic');

// This uses a high feature level to test with the latest version of the
// setTypingNotifier API.
final account = eg.account(
user: eg.selfUser, zulipFeatureLevel: eg.futureZulipFeatureLevel);

final contentInputFinder = find.byWidgetPredicate(
(widget) => widget is TextField && widget.controller is ComposeContentController);
final topicInputFinder = find.byWidgetPredicate(
(widget) => widget is TextField && widget.controller is ComposeTopicController);

void checkTypingRequest(TypingOp op, SendableNarrow narrow) =>
checkSetTypingStatusRequests(connection, [(op, narrow)]);

Future<void> checkStartTyping(WidgetTester tester, SendableNarrow narrow) async {
connection.prepare(json: {});
await tester.enterText(contentInputFinder, 'hello world');
checkTypingRequest(TypingOp.start, narrow);
}

testWidgets('smoke TopicNarrow', (tester) async {
await prepareComposeBox(tester, narrow: narrow, account: account);

await checkStartTyping(tester, narrow);

connection.prepare(json: {});
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
checkTypingRequest(TypingOp.stop, narrow);
});

testWidgets('smoke DmNarrow', (tester) async {
final narrow = DmNarrow.withUsers(
[eg.otherUser.userId], selfUserId: eg.selfUser.userId);
await prepareComposeBox(tester, narrow: narrow, account: account);

await checkStartTyping(tester, narrow);

connection.prepare(json: {});
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
checkTypingRequest(TypingOp.stop, narrow);
});

testWidgets('smoke ChannelNarrow', (tester) async {
const narrow = ChannelNarrow(123);
final destinationNarrow = TopicNarrow(narrow.streamId, 'test topic');
await prepareComposeBox(tester, narrow: narrow, account: account);

await tester.enterText(topicInputFinder, destinationNarrow.topic);
// Remove an irrelevant topic request.
check(connection.takeRequests()).single
..method.equals('GET')
..url.path.equals('/api/v1/users/me/123/topics');

await checkStartTyping(tester, destinationNarrow);

connection.prepare(json: {});
await tester.pump(store.typingNotifier.typingStoppedWaitPeriod);
checkTypingRequest(TypingOp.stop, destinationNarrow);
});

testWidgets('clearing text stops typing notification', (tester) async {
await prepareComposeBox(tester, narrow: narrow, account: account);

await checkStartTyping(tester, narrow);

connection.prepare(json: {});
await tester.enterText(contentInputFinder, '');
checkTypingRequest(TypingOp.stop, narrow);
});

Future<void> prepareComposeBoxWithNavigation(WidgetTester tester) async {
addTearDown(testBinding.reset);
await testBinding.globalStore.add(account, eg.initialSnapshot(
zulipFeatureLevel: account.zulipFeatureLevel));

await tester.pumpWidget(const ZulipApp());
await tester.pump();
final navigator = await ZulipApp.navigator;
navigator.push(MaterialAccountWidgetRoute(
accountId: account.id, page: const ComposeBox(narrow: narrow)));
await tester.pumpAndSettle();

store = await testBinding.globalStore.perAccount(account.id);
connection = store.connection as FakeApiConnection;
}

testWidgets('navigating away stops typing notification', (tester) async {
await prepareComposeBoxWithNavigation(tester);

await checkStartTyping(tester, narrow);

connection.prepare(json: {});
(await ZulipApp.navigator).pop();
await tester.pump(Duration.zero);
checkTypingRequest(TypingOp.stop, narrow);
});

testWidgets('for content input, unfocusing stops typing notification and refocusing does not start it', (tester) async {
const narrow = ChannelNarrow(123);
final destinationNarrow = TopicNarrow(narrow.streamId, 'test topic');
await prepareComposeBox(tester, narrow: narrow, account: account);

await tester.enterText(topicInputFinder, destinationNarrow.topic);
// Remove an irrelevant topic request.
check(connection.takeRequests()).single
..method.equals('GET')
..url.path.equals('/api/v1/users/me/123/topics');

connection.prepare(json: {});
await tester.enterText(contentInputFinder, 'hello world');
checkTypingRequest(TypingOp.start, destinationNarrow);

connection.prepare(json: {});
// Move focus to the topic input
await tester.tap(topicInputFinder);
await tester.pump(Duration.zero);
checkTypingRequest(TypingOp.stop, destinationNarrow);

await tester.tap(contentInputFinder);
await tester.pump(Duration.zero);
// Refocusing on the content input would trigger an update to
// [TextEditingController.selection]. Still, we expect no
// "typing started" request if the text remains the same.
});
});

group('message-send request response', () {
Future<void> setupAndTapSend(WidgetTester tester, {
required void Function(int messageId) prepareResponse,
Expand Down

0 comments on commit e97ffdc

Please sign in to comment.