Skip to content

Commit 4942f25

Browse files
gnpricechrisbobbe
authored andcommitted
autocomplete: Simplify updating input on topic autocomplete
The point in this autocomplete UI is to choose the entire new value for the topic input. So we can do that directly, without the more complicated logic that parallels what we need in the more complicated case of autocomplete in the content input. This is NFC as far as the text value of the input is concerned. I believe it's probably NFC entirely, because the remaining aspects of TextEditingValue -- the selection and the composing region -- are probably clobbered anyway by the next line's giving focus to the content input. (The new test already passes before this change.)
1 parent ad55496 commit 4942f25

File tree

3 files changed

+40
-7
lines changed

3 files changed

+40
-7
lines changed

lib/widgets/autocomplete.dart

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -321,13 +321,8 @@ class TopicAutocomplete extends AutocompleteField<TopicAutocompleteQuery, TopicA
321321
void _onTapOption(BuildContext context, TopicAutocompleteResult option) {
322322
final intent = autocompleteIntent();
323323
if (intent == null) return;
324-
final replacementString = option.topic;
325-
controller.value = intent.textEditingValue.replaced(
326-
TextRange(
327-
start: intent.syntaxStart,
328-
end: intent.textEditingValue.text.length),
329-
replacementString,
330-
);
324+
assert(intent.syntaxStart == 0);
325+
controller.value = TextEditingValue(text: option.topic);
331326
contentFocusNode.requestFocus();
332327
}
333328

test/flutter_checks.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ extension TextChecks on Subject<Text> {
6666
Subject<TextStyle?> get style => has((t) => t.style, 'style');
6767
}
6868

69+
extension TextEditingValueChecks on Subject<TextEditingValue> {
70+
Subject<String> get text => has((x) => x.text, 'text');
71+
Subject<TextSelection> get selection => has((x) => x.selection, 'selection');
72+
Subject<TextRange> get composing => has((x) => x.composing, 'composing');
73+
}
74+
6975
extension TextEditingControllerChecks on Subject<TextEditingController> {
7076
Subject<String?> get text => has((t) => t.text, 'text');
7177
}

test/widgets/autocomplete_test.dart

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import 'package:zulip/widgets/message_list.dart';
1717

1818
import '../api/fake_api.dart';
1919
import '../example_data.dart' as eg;
20+
import '../flutter_checks.dart';
2021
import '../model/binding.dart';
2122
import '../model/test_store.dart';
2223
import '../test_images.dart';
@@ -312,5 +313,36 @@ void main() {
312313
await tester.pumpAndSettle();
313314
checkTopicShown(topic2, store, expected: true);
314315
});
316+
317+
testWidgets('text selection is reset on choosing an option', (tester) async {
318+
// TODO test also that composing region gets reset.
319+
// (Just adding it to the updateEditingValue call below doesn't seem
320+
// to suffice to set it up; the controller value after the pump still
321+
// has empty composing region, so there's nothing to check after tap.)
322+
323+
final topic = eg.getStreamTopicsEntry(name: 'some topic');
324+
final topicInputFinder = await setupToTopicInput(tester, topics: [topic]);
325+
final controller = tester.widget<TextField>(topicInputFinder).controller!;
326+
327+
await tester.enterText(topicInputFinder, 'so');
328+
await tester.enterText(topicInputFinder, 'some');
329+
tester.testTextInput.updateEditingValue(const TextEditingValue(
330+
text: 'some',
331+
selection: TextSelection(baseOffset: 1, extentOffset: 3)));
332+
await tester.pump();
333+
check(controller.value)
334+
..text.equals('some')
335+
..selection.equals(
336+
const TextSelection(baseOffset: 1, extentOffset: 3));
337+
338+
await tester.tap(find.text('some topic'));
339+
await tester.pump();
340+
check(controller.value)
341+
..text.equals('some topic')
342+
..selection.equals(
343+
const TextSelection.collapsed(offset: 'some topic'.length));
344+
345+
await tester.pump(Duration.zero);
346+
});
315347
});
316348
}

0 commit comments

Comments
 (0)