From ababf79bd0f158d762537075de17d59c04fbca7e Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Tue, 30 Jul 2024 20:42:46 +0300 Subject: [PATCH 1/5] autocomplete [nfc]: Rename Autocomplete to ComposeContentAutocomplete The name Autocomplete is too generic and it collides with flutter's material Autocomplete widget name. So this uses the more specific name 'ComposeContentAutocomplete'. --- lib/model/autocomplete.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index f87e7650b4..ec6bcf64ca 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -7,7 +7,7 @@ import '../widgets/compose_box.dart'; import 'narrow.dart'; import 'store.dart'; -extension Autocomplete on ComposeContentController { +extension ComposeContentAutocomplete on ComposeContentController { AutocompleteIntent? autocompleteIntent() { if (!selection.isValid || !selection.isNormalized) { // We don't require [isCollapsed] to be true because we've seen that From 0ce17ada5d5280565ee982bb39a870e2bfb2462a Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Fri, 2 Aug 2024 07:50:30 +0300 Subject: [PATCH 2/5] autocomplete [nfc]: Factor out generic AutocompleteView The mechanism of query and results is generic to the idea of autocomplete in general, it's not specific to autocomplete on @-mentions vs. topics vs. anything else. --- lib/model/autocomplete.dart | 209 +++++++++++++++------------ test/model/autocomplete_checks.dart | 6 +- test/widgets/compose_box_checks.dart | 2 +- 3 files changed, 124 insertions(+), 93 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index ec6bcf64ca..8beb188720 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -8,7 +8,7 @@ import 'narrow.dart'; import 'store.dart'; extension ComposeContentAutocomplete on ComposeContentController { - AutocompleteIntent? autocompleteIntent() { + AutocompleteIntent? autocompleteIntent() { if (!selection.isValid || !selection.isNormalized) { // We don't require [isCollapsed] to be true because we've seen that // autocorrect and even backspace involve programmatically expanding the @@ -68,8 +68,8 @@ final RegExp mentionAutocompleteMarkerRegex = (() { unicode: true); })(); -/// The content controller's recognition that the user might want autocomplete UI. -class AutocompleteIntent { +/// The text controller's recognition that the user might want autocomplete UI. +class AutocompleteIntent { AutocompleteIntent({ required this.syntaxStart, required this.query, @@ -91,7 +91,7 @@ class AutocompleteIntent { // that use a custom/subclassed [TextEditingValue], so that's not convenient. final int syntaxStart; - final MentionAutocompleteQuery query; // TODO other autocomplete query types + final QueryT query; /// The [TextEditingValue] whose text [syntaxStart] refers to. final TextEditingValue textEditingValue; @@ -151,21 +151,90 @@ class AutocompleteViewManager { // void dispose() { … } } -/// A view-model for a mention-autocomplete interaction. +/// A view-model for an autocomplete interaction. /// /// The owner of one of these objects must call [dispose] when the object /// will no longer be used, in order to free resources on the [PerAccountStore]. /// /// Lifecycle: -/// * Create with [init]. +/// * Create an instance of a concrete subtype. /// * Add listeners with [addListener]. /// * Use the [query] setter to start a search for a query. /// * On reassemble, call [reassemble]. /// * When the object will no longer be used, call [dispose] to free /// resources on the [PerAccountStore]. -class MentionAutocompleteView extends ChangeNotifier { +abstract class AutocompleteView extends ChangeNotifier { + AutocompleteView({required this.store}); + + final PerAccountStore store; + + Iterable getSortedItemsToTest(QueryT query); + + ResultT? testItem(QueryT query, CandidateT item); + + QueryT? get query => _query; + QueryT? _query; + set query(QueryT? query) { + _query = query; + if (query != null) { + _startSearch(query); + } + } + + /// Called when the app is reassembled during debugging, e.g. for hot reload. + /// + /// This will redo the search from scratch for the current query, if any. + void reassemble() { + if (_query != null) { + _startSearch(_query!); + } + } + + Iterable get results => _results; + List _results = []; + + Future _startSearch(QueryT query) async { + final newResults = await _computeResults(query); + if (newResults == null) { + // Query was old; new search is in progress. Or, no listeners to notify. + return; + } + + _results = newResults; + notifyListeners(); + } + + Future?> _computeResults(QueryT query) async { + final List results = []; + final Iterable data = getSortedItemsToTest(query); + + final iterator = data.iterator; + bool isDone = false; + while (!isDone) { + // CPU perf: End this task; enqueue a new one for resuming this work + await Future(() {}); + + if (query != _query || !hasListeners) { // false if [dispose] has been called. + return null; + } + + for (int i = 0; i < 1000; i++) { + if (!iterator.moveNext()) { + isDone = true; + break; + } + final CandidateT item = iterator.current; + final result = testItem(query, item); + if (result != null) results.add(result); + } + } + return results; + } +} + +class MentionAutocompleteView extends AutocompleteView { MentionAutocompleteView._({ - required this.store, + required super.store, required this.narrow, required this.sortedUsers, }); @@ -183,6 +252,9 @@ class MentionAutocompleteView extends ChangeNotifier { return view; } + final Narrow narrow; + final List sortedUsers; + static List _usersByRelevance({ required PerAccountStore store, required Narrow narrow, @@ -289,6 +361,19 @@ class MentionAutocompleteView extends ChangeNotifier { streamId: streamId, senderId: userB.userId)); } + @override + Iterable getSortedItemsToTest(MentionAutocompleteQuery query) { + return sortedUsers; + } + + @override + MentionAutocompleteResult? testItem(MentionAutocompleteQuery query, User item) { + if (query.testUser(item, store.autocompleteViewManager.autocompleteDataCache)) { + return UserMentionAutocompleteResult(userId: item.userId); + } + return null; + } + /// Determines which of the two users is more recent in DM conversations. /// /// Returns a negative number if [userA] is more recent than [userB], @@ -349,82 +434,44 @@ class MentionAutocompleteView extends ChangeNotifier { // TODO test that logic (may involve detecting an unhandled Future rejection; how?) super.dispose(); } +} - final PerAccountStore store; - final Narrow narrow; - final List sortedUsers; +abstract class AutocompleteQuery { + AutocompleteQuery(this.raw) + : _lowercaseWords = raw.toLowerCase().split(' '); - MentionAutocompleteQuery? get query => _query; - MentionAutocompleteQuery? _query; - set query(MentionAutocompleteQuery? query) { - _query = query; - if (query != null) { - _startSearch(query); - } - } + final String raw; + final List _lowercaseWords; - /// Called when the app is reassembled during debugging, e.g. for hot reload. + /// Whether all of this query's words have matches in [words] that appear in order. /// - /// This will redo the search from scratch for the current query, if any. - void reassemble() { - if (_query != null) { - _startSearch(_query!); - } - } - - Iterable get results => _results; - List _results = []; - - Future _startSearch(MentionAutocompleteQuery query) async { - final newResults = await _computeResults(query); - if (newResults == null) { - // Query was old; new search is in progress. Or, no listeners to notify. - return; - } - - _results = newResults; - notifyListeners(); - } - - Future?> _computeResults(MentionAutocompleteQuery query) async { - final List results = []; - final iterator = sortedUsers.iterator; - bool isDone = false; - while (!isDone) { - // CPU perf: End this task; enqueue a new one for resuming this work - await Future(() {}); - - if (query != _query || !hasListeners) { // false if [dispose] has been called. - return null; + /// A "match" means the word in [words] starts with the query word. + bool _testContainsQueryWords(List words) { + // TODO(#237) test with diacritics stripped, where appropriate + int wordsIndex = 0; + int queryWordsIndex = 0; + while (true) { + if (queryWordsIndex == _lowercaseWords.length) { + return true; + } + if (wordsIndex == words.length) { + return false; } - for (int i = 0; i < 1000; i++) { - if (!iterator.moveNext()) { - isDone = true; - break; - } - - final User user = iterator.current; - if (query.testUser(user, store.autocompleteViewManager.autocompleteDataCache)) { - results.add(UserMentionAutocompleteResult(userId: user.userId)); - } + if (words[wordsIndex].startsWith(_lowercaseWords[queryWordsIndex])) { + queryWordsIndex++; } + wordsIndex++; } - return results; } } -class MentionAutocompleteQuery { - MentionAutocompleteQuery(this.raw, {this.silent = false}) - : _lowercaseWords = raw.toLowerCase().split(' '); - - final String raw; +class MentionAutocompleteQuery extends AutocompleteQuery { + MentionAutocompleteQuery(super.raw, {this.silent = false}); /// Whether the user wants a silent mention (@_query, vs. @query). final bool silent; - final List _lowercaseWords; - bool testUser(User user, AutocompleteDataCache cache) { // TODO(#236) test email too, not just name @@ -434,25 +481,7 @@ class MentionAutocompleteQuery { } bool _testName(User user, AutocompleteDataCache cache) { - // TODO(#237) test with diacritics stripped, where appropriate - - final List nameWords = cache.nameWordsForUser(user); - - int nameWordsIndex = 0; - int queryWordsIndex = 0; - while (true) { - if (queryWordsIndex == _lowercaseWords.length) { - return true; - } - if (nameWordsIndex == nameWords.length) { - return false; - } - - if (nameWords[nameWordsIndex].startsWith(_lowercaseWords[queryWordsIndex])) { - queryWordsIndex++; - } - nameWordsIndex++; - } + return _testContainsQueryWords(cache.nameWordsForUser(user)); } @override @@ -489,7 +518,9 @@ class AutocompleteDataCache { } } -sealed class MentionAutocompleteResult {} +class AutocompleteResult {} + +sealed class MentionAutocompleteResult extends AutocompleteResult {} class UserMentionAutocompleteResult extends MentionAutocompleteResult { UserMentionAutocompleteResult({required this.userId}); diff --git a/test/model/autocomplete_checks.dart b/test/model/autocomplete_checks.dart index 4b7e3c8bad..3b42a3fa18 100644 --- a/test/model/autocomplete_checks.dart +++ b/test/model/autocomplete_checks.dart @@ -3,12 +3,12 @@ import 'package:zulip/model/autocomplete.dart'; import 'package:zulip/widgets/compose_box.dart'; extension ComposeContentControllerChecks on Subject { - Subject get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent'); + Subject?> get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent'); } -extension AutocompleteIntentChecks on Subject { +extension AutocompleteIntentChecks on Subject> { Subject get syntaxStart => has((i) => i.syntaxStart, 'syntaxStart'); - Subject get query => has((i) => i.query, 'query'); + Subject get query => has((i) => i.query, 'query'); } extension UserMentionAutocompleteResultChecks on Subject { diff --git a/test/widgets/compose_box_checks.dart b/test/widgets/compose_box_checks.dart index 03aabbe8ee..ae351a2922 100644 --- a/test/widgets/compose_box_checks.dart +++ b/test/widgets/compose_box_checks.dart @@ -8,7 +8,7 @@ extension ComposeContentControllerChecks on Subject { extension AutocompleteIntentChecks on Subject { Subject get syntaxStart => has((i) => i.syntaxStart, 'syntaxStart'); - Subject get query => has((i) => i.query, 'query'); + Subject get query => has((i) => i.query, 'query'); } extension UserMentionAutocompleteResultChecks on Subject { From fad368c01e73df16a6c3b7eefa0f8e8727f58d01 Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Sat, 10 Aug 2024 16:29:46 +0300 Subject: [PATCH 3/5] autocomplete [nfc]: Factor out generic AutocompleteField Most of the logic in `ComposeAutocomplete` is not specific to the content input it self rather it is general logic that applies to any autocomplete field. --- lib/widgets/autocomplete.dart | 172 ++++++++++++++++++++-------------- 1 file changed, 101 insertions(+), 71 deletions(-) diff --git a/lib/widgets/autocomplete.dart b/lib/widgets/autocomplete.dart index ece46a326f..e2cc6e0a9a 100644 --- a/lib/widgets/autocomplete.dart +++ b/lib/widgets/autocomplete.dart @@ -1,5 +1,6 @@ import 'package:flutter/material.dart'; +import '../api/model/model.dart'; import 'content.dart'; import 'store.dart'; import '../model/autocomplete.dart'; @@ -7,37 +8,38 @@ import '../model/compose.dart'; import '../model/narrow.dart'; import 'compose_box.dart'; -class ComposeAutocomplete extends StatefulWidget { - const ComposeAutocomplete({ +abstract class AutocompleteField extends StatefulWidget { + const AutocompleteField({ super.key, - required this.narrow, required this.controller, required this.focusNode, required this.fieldViewBuilder, }); - /// The message list's narrow. - final Narrow narrow; - - final ComposeContentController controller; + final TextEditingController controller; final FocusNode focusNode; final WidgetBuilder fieldViewBuilder; + AutocompleteIntent? autocompleteIntent(); + + Widget buildItem(BuildContext context, int index, ResultT option); + + AutocompleteView initViewModel(BuildContext context); + @override - State createState() => _ComposeAutocompleteState(); + State> createState() => _AutocompleteFieldState(); } -class _ComposeAutocompleteState extends State with PerAccountStoreAwareStateMixin { - MentionAutocompleteView? _viewModel; // TODO different autocomplete view types +class _AutocompleteFieldState extends State> with PerAccountStoreAwareStateMixin> { + AutocompleteView? _viewModel; void _initViewModel() { - final store = PerAccountStoreWidget.of(context); - _viewModel = MentionAutocompleteView.init(store: store, narrow: widget.narrow) + _viewModel = widget.initViewModel(context) ..addListener(_viewModelChanged); } - void _composeContentChanged() { - final newAutocompleteIntent = widget.controller.autocompleteIntent(); + void _handleControllerChange() { + final newAutocompleteIntent = widget.autocompleteIntent(); if (newAutocompleteIntent != null) { if (_viewModel == null) { _initViewModel(); @@ -55,7 +57,7 @@ class _ComposeAutocompleteState extends State with PerAccou @override void initState() { super.initState(); - widget.controller.addListener(_composeContentChanged); + widget.controller.addListener(_handleControllerChange); } @override @@ -69,22 +71,22 @@ class _ComposeAutocompleteState extends State with PerAccou } @override - void didUpdateWidget(covariant ComposeAutocomplete oldWidget) { + void didUpdateWidget(covariant AutocompleteField oldWidget) { super.didUpdateWidget(oldWidget); if (widget.controller != oldWidget.controller) { - oldWidget.controller.removeListener(_composeContentChanged); - widget.controller.addListener(_composeContentChanged); + oldWidget.controller.removeListener(_handleControllerChange); + widget.controller.addListener(_handleControllerChange); } } @override void dispose() { - widget.controller.removeListener(_composeContentChanged); + widget.controller.removeListener(_handleControllerChange); _viewModel?.dispose(); // removes our listener super.dispose(); } - List _resultsToDisplay = []; + List _resultsToDisplay = []; void _viewModelChanged() { setState(() { @@ -92,65 +94,20 @@ class _ComposeAutocompleteState extends State with PerAccou }); } - void _onTapOption(MentionAutocompleteResult option) { - // Probably the same intent that brought up the option that was tapped. - // If not, it still shouldn't be off by more than the time it takes - // to compute the autocomplete results, which we do asynchronously. - final intent = widget.controller.autocompleteIntent(); - if (intent == null) { - return; // Shrug. - } - - final store = PerAccountStoreWidget.of(context); - final String replacementString; - switch (option) { - case UserMentionAutocompleteResult(:var userId): - // TODO(i18n) language-appropriate space character; check active keyboard? - // (maybe handle centrally in `widget.controller`) - replacementString = '${mention(store.users[userId]!, silent: intent.query.silent, users: store.users)} '; - } - - widget.controller.value = intent.textEditingValue.replaced( - TextRange( - start: intent.syntaxStart, - end: intent.textEditingValue.selection.end), - replacementString, - ); - } - - Widget _buildItem(BuildContext _, int index) { - final option = _resultsToDisplay[index]; - Widget avatar; - String label; - switch (option) { - case UserMentionAutocompleteResult(:var userId): - avatar = Avatar(userId: userId, size: 32, borderRadius: 3); - label = PerAccountStoreWidget.of(context).users[userId]!.fullName; - } - return InkWell( - onTap: () { - _onTapOption(option); - }, - child: Padding( - padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0), - child: Row( - children: [ - avatar, - const SizedBox(width: 8), - Text(label), - ]))); + Widget _buildItem(BuildContext context, int index) { + return widget.buildItem(context, index, _resultsToDisplay[index]); } @override Widget build(BuildContext context) { - return RawAutocomplete( + return RawAutocomplete( textEditingController: widget.controller, focusNode: widget.focusNode, optionsBuilder: (_) => _resultsToDisplay, optionsViewOpenDirection: OptionsViewOpenDirection.up, // RawAutocomplete passes these when it calls optionsViewBuilder: - // AutocompleteOnSelected onSelected, - // Iterable options, + // AutocompleteOnSelected onSelected, + // Iterable options, // // We ignore them: // - `onSelected` would cause some behavior we don't want, @@ -159,7 +116,7 @@ class _ComposeAutocompleteState extends State with PerAccou // the work of creating the list of options. We're not; the // `optionsBuilder` we pass is just a function that returns // _resultsToDisplay, which is computed with lots of help from - // MentionAutocompleteView. + // AutocompleteView. optionsViewBuilder: (context, _, __) { return Align( alignment: Alignment.bottomLeft, @@ -187,3 +144,76 @@ class _ComposeAutocompleteState extends State with PerAccou ); } } + +class ComposeAutocomplete extends AutocompleteField { + const ComposeAutocomplete({ + super.key, + required this.narrow, + required super.focusNode, + required super.fieldViewBuilder, + required ComposeContentController super.controller, + }); + + final Narrow narrow; + + @override + ComposeContentController get controller => super.controller as ComposeContentController; + + @override + AutocompleteIntent? autocompleteIntent() => controller.autocompleteIntent(); + + @override + MentionAutocompleteView initViewModel(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + return MentionAutocompleteView.init(store: store, narrow: narrow); + } + + void _onTapOption(BuildContext context, MentionAutocompleteResult option) { + // Probably the same intent that brought up the option that was tapped. + // If not, it still shouldn't be off by more than the time it takes + // to compute the autocomplete results, which we do asynchronously. + final intent = autocompleteIntent(); + if (intent == null) { + return; // Shrug. + } + + final store = PerAccountStoreWidget.of(context); + final String replacementString; + switch (option) { + case UserMentionAutocompleteResult(:var userId): + // TODO(i18n) language-appropriate space character; check active keyboard? + // (maybe handle centrally in `controller`) + replacementString = '${mention(store.users[userId]!, silent: intent.query.silent, users: store.users)} '; + } + + controller.value = intent.textEditingValue.replaced( + TextRange( + start: intent.syntaxStart, + end: intent.textEditingValue.selection.end), + replacementString, + ); + } + + @override + Widget buildItem(BuildContext context, int index, MentionAutocompleteResult option) { + Widget avatar; + String label; + switch (option) { + case UserMentionAutocompleteResult(:var userId): + avatar = Avatar(userId: userId, size: 32, borderRadius: 3); + label = PerAccountStoreWidget.of(context).users[userId]!.fullName; + } + return InkWell( + onTap: () { + _onTapOption(context, option); + }, + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0), + child: Row( + children: [ + avatar, + const SizedBox(width: 8), + Text(label), + ]))); + } +} From 50a7609ac5bd706d6d89bf2cd0862280e336e824 Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Sat, 10 Aug 2024 16:32:58 +0300 Subject: [PATCH 4/5] api: Add route getStreamTopics --- lib/api/route/channels.dart | 40 +++++++++++++++++++++++++++++++++++ lib/api/route/channels.g.dart | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) create mode 100644 lib/api/route/channels.dart create mode 100644 lib/api/route/channels.g.dart diff --git a/lib/api/route/channels.dart b/lib/api/route/channels.dart new file mode 100644 index 0000000000..d03ce501cf --- /dev/null +++ b/lib/api/route/channels.dart @@ -0,0 +1,40 @@ +import 'package:json_annotation/json_annotation.dart'; + +import '../core.dart'; +part 'channels.g.dart'; + +/// https://zulip.com/api/get-stream-topics +Future getStreamTopics(ApiConnection connection, { + required int streamId, +}) { + return connection.get('getStreamTopics', GetStreamTopicsResult.fromJson, 'users/me/$streamId/topics', {}); +} + +@JsonSerializable(fieldRename: FieldRename.snake) +class GetStreamTopicsResult { + final List topics; + + GetStreamTopicsResult({ + required this.topics, + }); + + factory GetStreamTopicsResult.fromJson(Map json) => + _$GetStreamTopicsResultFromJson(json); + + Map toJson() => _$GetStreamTopicsResultToJson(this); +} + +@JsonSerializable(fieldRename: FieldRename.snake) +class GetStreamTopicsEntry { + final int maxId; + final String name; + + GetStreamTopicsEntry({ + required this.maxId, + required this.name, + }); + + factory GetStreamTopicsEntry.fromJson(Map json) => _$GetStreamTopicsEntryFromJson(json); + + Map toJson() => _$GetStreamTopicsEntryToJson(this); +} diff --git a/lib/api/route/channels.g.dart b/lib/api/route/channels.g.dart new file mode 100644 index 0000000000..561b43f005 --- /dev/null +++ b/lib/api/route/channels.g.dart @@ -0,0 +1,37 @@ +// GENERATED CODE - DO NOT MODIFY BY HAND + +// ignore_for_file: constant_identifier_names, unnecessary_cast + +part of 'channels.dart'; + +// ************************************************************************** +// JsonSerializableGenerator +// ************************************************************************** + +GetStreamTopicsResult _$GetStreamTopicsResultFromJson( + Map json) => + GetStreamTopicsResult( + topics: (json['topics'] as List) + .map((e) => GetStreamTopicsEntry.fromJson(e as Map)) + .toList(), + ); + +Map _$GetStreamTopicsResultToJson( + GetStreamTopicsResult instance) => + { + 'topics': instance.topics, + }; + +GetStreamTopicsEntry _$GetStreamTopicsEntryFromJson( + Map json) => + GetStreamTopicsEntry( + maxId: (json['max_id'] as num).toInt(), + name: json['name'] as String, + ); + +Map _$GetStreamTopicsEntryToJson( + GetStreamTopicsEntry instance) => + { + 'max_id': instance.maxId, + 'name': instance.name, + }; From 046ceabfd4a0610e7f2e32bfb0fa26bc337923bc Mon Sep 17 00:00:00 2001 From: Khader M Khudair Date: Mon, 12 Aug 2024 12:43:13 +0300 Subject: [PATCH 5/5] autocomplete: Add autocomplete for topic to send to The filtering logic (in [TopicAutocompleteQuery.testTopic]) differs from zulip-mobile but agrees with web. Details at: https://github.com/zulip/zulip-flutter/pull/627#discussion_r1713064079 Fixes: #310 --- lib/model/autocomplete.dart | 101 +++++++++++++++++- lib/widgets/autocomplete.dart | 51 ++++++++++ lib/widgets/compose_box.dart | 45 ++++++-- test/example_data.dart | 6 ++ test/model/autocomplete_checks.dart | 8 ++ test/model/autocomplete_test.dart | 152 ++++++++++++++++++++++------ test/widgets/action_sheet_test.dart | 7 ++ test/widgets/autocomplete_test.dart | 81 +++++++++++++++ test/widgets/compose_box_test.dart | 6 ++ 9 files changed, 418 insertions(+), 39 deletions(-) diff --git a/lib/model/autocomplete.dart b/lib/model/autocomplete.dart index 8beb188720..cf9a9505a5 100644 --- a/lib/model/autocomplete.dart +++ b/lib/model/autocomplete.dart @@ -3,6 +3,7 @@ import 'package:flutter/services.dart'; import '../api/model/events.dart'; import '../api/model/model.dart'; +import '../api/route/channels.dart'; import '../widgets/compose_box.dart'; import 'narrow.dart'; import 'store.dart'; @@ -43,6 +44,15 @@ extension ComposeContentAutocomplete on ComposeContentController { } } +extension ComposeTopicAutocomplete on ComposeTopicController { + AutocompleteIntent? autocompleteIntent() { + return AutocompleteIntent( + syntaxStart: 0, + query: TopicAutocompleteQuery(value.text), + textEditingValue: value); + } +} + final RegExp mentionAutocompleteMarkerRegex = (() { // What's likely to come before an @-mention: the start of the string, // whitespace, or punctuation. Letters are unlikely; in that case an email @@ -112,6 +122,7 @@ class AutocompleteIntent { /// On reassemble, call [reassemble]. class AutocompleteViewManager { final Set _mentionAutocompleteViews = {}; + final Set _topicAutocompleteViews = {}; AutocompleteDataCache autocompleteDataCache = AutocompleteDataCache(); @@ -125,6 +136,16 @@ class AutocompleteViewManager { assert(removed); } + void registerTopicAutocomplete(TopicAutocompleteView view) { + final added = _topicAutocompleteViews.add(view); + assert(added); + } + + void unregisterTopicAutocomplete(TopicAutocompleteView view) { + final removed = _topicAutocompleteViews.remove(view); + assert(removed); + } + void handleRealmUserRemoveEvent(RealmUserRemoveEvent event) { autocompleteDataCache.invalidateUser(event.userId); } @@ -135,12 +156,15 @@ class AutocompleteViewManager { /// Called when the app is reassembled during debugging, e.g. for hot reload. /// - /// Calls [MentionAutocompleteView.reassemble] for all that are registered. + /// Calls [AutocompleteView.reassemble] for all that are registered. /// void reassemble() { for (final view in _mentionAutocompleteViews) { view.reassemble(); } + for (final view in _topicAutocompleteViews) { + view.reassemble(); + } } // No `dispose` method, because there's nothing for it to do. @@ -531,3 +555,78 @@ class UserMentionAutocompleteResult extends MentionAutocompleteResult { // TODO(#233): // class UserGroupMentionAutocompleteResult extends MentionAutocompleteResult { // TODO(#234): // class WildcardMentionAutocompleteResult extends MentionAutocompleteResult { + +class TopicAutocompleteView extends AutocompleteView { + TopicAutocompleteView._({required super.store, required this.streamId}); + + factory TopicAutocompleteView.init({required PerAccountStore store, required int streamId}) { + final view = TopicAutocompleteView._(store: store, streamId: streamId); + store.autocompleteViewManager.registerTopicAutocomplete(view); + view._fetch(); + return view; + } + + final int streamId; + Iterable _topics = []; + bool _isFetching = false; + + /// Fetches topics of the current stream narrow, expected to fetch + /// only once per lifecycle. + /// + /// Starts fetching once the stream narrow is active, then when results + /// are fetched it restarts search to refresh UI showing the newly + /// fetched topics. + Future _fetch() async { + assert(!_isFetching); + _isFetching = true; + final result = await getStreamTopics(store.connection, streamId: streamId); + _topics = result.topics.map((e) => e.name); + _isFetching = false; + if (_query != null) _startSearch(_query!); + } + + @override + Iterable getSortedItemsToTest(TopicAutocompleteQuery query) => _topics; + + @override + TopicAutocompleteResult? testItem(TopicAutocompleteQuery query, String item) { + if (query.testTopic(item)) { + return TopicAutocompleteResult(topic: item); + } + return null; + } + + @override + void dispose() { + store.autocompleteViewManager.unregisterTopicAutocomplete(this); + super.dispose(); + } +} + +class TopicAutocompleteQuery extends AutocompleteQuery { + TopicAutocompleteQuery(super.raw); + + bool testTopic(String topic) { + // TODO(#881): Sort by match relevance, like web does. + return topic != raw && topic.toLowerCase().contains(raw.toLowerCase()); + } + + @override + String toString() { + return '${objectRuntimeType(this, 'TopicAutocompleteQuery')}(raw: $raw)'; + } + + @override + bool operator ==(Object other) { + return other is TopicAutocompleteQuery && other.raw == raw; + } + + @override + int get hashCode => Object.hash('TopicAutocompleteQuery', raw); +} + +class TopicAutocompleteResult extends AutocompleteResult { + final String topic; + + TopicAutocompleteResult({required this.topic}); +} diff --git a/lib/widgets/autocomplete.dart b/lib/widgets/autocomplete.dart index e2cc6e0a9a..c1e82a5954 100644 --- a/lib/widgets/autocomplete.dart +++ b/lib/widgets/autocomplete.dart @@ -217,3 +217,54 @@ class ComposeAutocomplete extends AutocompleteField { + const TopicAutocomplete({ + super.key, + required this.streamId, + required ComposeTopicController super.controller, + required super.focusNode, + required this.contentFocusNode, + required super.fieldViewBuilder, + }); + + final FocusNode contentFocusNode; + + final int streamId; + + @override + ComposeTopicController get controller => super.controller as ComposeTopicController; + + @override + AutocompleteIntent? autocompleteIntent() => controller.autocompleteIntent(); + + @override + TopicAutocompleteView initViewModel(BuildContext context) { + final store = PerAccountStoreWidget.of(context); + return TopicAutocompleteView.init(store: store, streamId: streamId); + } + + void _onTapOption(BuildContext context, TopicAutocompleteResult option) { + final intent = autocompleteIntent(); + if (intent == null) return; + final replacementString = option.topic; + controller.value = intent.textEditingValue.replaced( + TextRange( + start: intent.syntaxStart, + end: intent.textEditingValue.text.length), + replacementString, + ); + contentFocusNode.requestFocus(); + } + + @override + Widget buildItem(BuildContext context, int index, TopicAutocompleteResult option) { + return InkWell( + onTap: () { + _onTapOption(context, option); + }, + child: Padding( + padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0), + child: Text(option.topic))); + } +} diff --git a/lib/widgets/compose_box.dart b/lib/widgets/compose_box.dart index 1c62bc237b..4897b88629 100644 --- a/lib/widgets/compose_box.dart +++ b/lib/widgets/compose_box.dart @@ -376,6 +376,38 @@ class _StreamContentInputState extends State<_StreamContentInput> { } } +class _TopicInput extends StatelessWidget { + const _TopicInput({ + required this.streamId, + required this.controller, + required this.focusNode, + required this.contentFocusNode}); + + final int streamId; + final ComposeTopicController controller; + final FocusNode focusNode; + final FocusNode contentFocusNode; + + @override + Widget build(BuildContext context) { + final zulipLocalizations = ZulipLocalizations.of(context); + ColorScheme colorScheme = Theme.of(context).colorScheme; + + return TopicAutocomplete( + streamId: streamId, + controller: controller, + focusNode: focusNode, + contentFocusNode: contentFocusNode, + fieldViewBuilder: (context) => TextField( + controller: controller, + focusNode: focusNode, + textInputAction: TextInputAction.next, + style: TextStyle(color: colorScheme.onSurface), + decoration: InputDecoration(hintText: zulipLocalizations.composeBoxTopicHintText), + )); + } +} + class _FixedDestinationContentInput extends StatelessWidget { const _FixedDestinationContentInput({ required this.narrow, @@ -956,6 +988,9 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose @override FocusNode get contentFocusNode => _contentFocusNode; final _contentFocusNode = FocusNode(); + FocusNode get topicFocusNode => _topicFocusNode; + final _topicFocusNode = FocusNode(); + @override void dispose() { _topicController.dispose(); @@ -966,16 +1001,14 @@ class _StreamComposeBoxState extends State<_StreamComposeBox> implements Compose @override Widget build(BuildContext context) { - final colorScheme = Theme.of(context).colorScheme; - final zulipLocalizations = ZulipLocalizations.of(context); - return _ComposeBoxLayout( contentController: _contentController, contentFocusNode: _contentFocusNode, - topicInput: TextField( + topicInput: _TopicInput( + streamId: widget.narrow.streamId, controller: _topicController, - style: TextStyle(color: colorScheme.onSurface), - decoration: InputDecoration(hintText: zulipLocalizations.composeBoxTopicHintText), + focusNode: topicFocusNode, + contentFocusNode: _contentFocusNode, ), contentInput: _StreamContentInput( narrow: widget.narrow, diff --git a/test/example_data.dart b/test/example_data.dart index aa4f1931e4..5ff4bc80ef 100644 --- a/test/example_data.dart +++ b/test/example_data.dart @@ -4,6 +4,7 @@ import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/realm.dart'; +import 'package:zulip/api/route/channels.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; @@ -204,6 +205,11 @@ ZulipStream stream({ } const _stream = stream; +GetStreamTopicsEntry getStreamTopicsEntry({int? maxId, String? name}) { + maxId ??= 123; + return GetStreamTopicsEntry(maxId: maxId, name: name ?? 'Test Topic #$maxId'); +} + /// Construct an example subscription from a stream. /// /// We only allow overrides of values specific to the [Subscription], all diff --git a/test/model/autocomplete_checks.dart b/test/model/autocomplete_checks.dart index 3b42a3fa18..93c4dbe196 100644 --- a/test/model/autocomplete_checks.dart +++ b/test/model/autocomplete_checks.dart @@ -6,6 +6,10 @@ extension ComposeContentControllerChecks on Subject { Subject?> get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent'); } +extension ComposeTopicControllerChecks on Subject { + Subject?> get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent'); +} + extension AutocompleteIntentChecks on Subject> { Subject get syntaxStart => has((i) => i.syntaxStart, 'syntaxStart'); Subject get query => has((i) => i.query, 'query'); @@ -14,3 +18,7 @@ extension AutocompleteIntentChecks on Subject { Subject get userId => has((r) => r.userId, 'userId'); } + +extension TopicAutocompleteResultChecks on Subject { + Subject get topic => has((r) => r.topic, 'topic'); +} diff --git a/test/model/autocomplete_test.dart b/test/model/autocomplete_test.dart index 3938b32a07..df68b58da1 100644 --- a/test/model/autocomplete_test.dart +++ b/test/model/autocomplete_test.dart @@ -7,11 +7,13 @@ import 'package:flutter/widgets.dart'; import 'package:test/scaffolding.dart'; import 'package:zulip/api/model/initial_snapshot.dart'; import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/route/channels.dart'; import 'package:zulip/model/autocomplete.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/compose_box.dart'; +import '../api/fake_api.dart'; import '../example_data.dart' as eg; import 'test_store.dart'; import 'autocomplete_checks.dart'; @@ -19,42 +21,42 @@ import 'autocomplete_checks.dart'; typedef MarkedTextParse = ({int? expectedSyntaxStart, TextEditingValue value}); void main() { - group('ComposeContentController.autocompleteIntent', () { - MarkedTextParse parseMarkedText(String markedText) { - final TextSelection selection; - int? expectedSyntaxStart; - final textBuffer = StringBuffer(); - final caretPositions = []; - int i = 0; - for (final char in markedText.codeUnits) { - if (char == 94 /* ^ */) { - caretPositions.add(i); - continue; - } else if (char == 126 /* ~ */) { - if (expectedSyntaxStart != null) { - throw Exception('Test error: too many ~ in input'); - } - expectedSyntaxStart = i; - continue; + ({int? expectedSyntaxStart, TextEditingValue value}) parseMarkedText(String markedText) { + final TextSelection selection; + int? expectedSyntaxStart; + final textBuffer = StringBuffer(); + final caretPositions = []; + int i = 0; + for (final char in markedText.codeUnits) { + if (char == 94 /* ^ */) { + caretPositions.add(i); + continue; + } else if (char == 126 /* ~ */) { + if (expectedSyntaxStart != null) { + throw Exception('Test error: too many ~ in input'); } - textBuffer.writeCharCode(char); - i++; - } - switch (caretPositions.length) { - case 0: - selection = const TextSelection.collapsed(offset: -1); - case 1: - selection = TextSelection(baseOffset: caretPositions[0], extentOffset: caretPositions[0]); - case 2: - selection = TextSelection(baseOffset: caretPositions[0], extentOffset: caretPositions[1]); - default: - throw Exception('Test error: too many ^ in input'); + expectedSyntaxStart = i; + continue; } - return ( - value: TextEditingValue(text: textBuffer.toString(), selection: selection), - expectedSyntaxStart: expectedSyntaxStart); + textBuffer.writeCharCode(char); + i++; } + switch (caretPositions.length) { + case 0: + selection = const TextSelection.collapsed(offset: -1); + case 1: + selection = TextSelection(baseOffset: caretPositions[0], extentOffset: caretPositions[0]); + case 2: + selection = TextSelection(baseOffset: caretPositions[0], extentOffset: caretPositions[1]); + default: + throw Exception('Test error: too many ^ in input'); + } + return ( + value: TextEditingValue(text: textBuffer.toString(), selection: selection), + expectedSyntaxStart: expectedSyntaxStart); + } + group('ComposeContentController.autocompleteIntent', () { /// Test the given input, in a convenient format. /// /// Represent selection handles as "^". For convenience, a single "^" can @@ -179,6 +181,7 @@ void main() { view.addListener(() { done = true; }); view.query = MentionAutocompleteQuery('Third'); await Future(() {}); + await Future(() {}); check(done).isTrue(); check(view.results).single .isA() @@ -732,4 +735,89 @@ void main() { .deepEquals([5, 4]); }); }); + + group('ComposeTopicAutocomplete.autocompleteIntent', () { + void doTest(String markedText, TopicAutocompleteQuery? expectedQuery) { + final parsed = parseMarkedText(markedText); + + final description = 'topic-input with text: $markedText produces: ${expectedQuery?.raw ?? 'No Query!'}'; + test(description, () { + final controller = ComposeTopicController(); + controller.value = parsed.value; + if (expectedQuery == null) { + check(controller).autocompleteIntent.isNull(); + } else { + check(controller).autocompleteIntent.isNotNull() + ..query.equals(expectedQuery) + ..syntaxStart.equals(0); // query is the whole value + } + }); + } + + /// if there is any input, produced query should match input text + doTest('', TopicAutocompleteQuery('')); + doTest('^abc', TopicAutocompleteQuery('abc')); + doTest('a^bc', TopicAutocompleteQuery('abc')); + doTest('abc^', TopicAutocompleteQuery('abc')); + doTest('a^bc^', TopicAutocompleteQuery('abc')); + }); + + test('TopicAutocompleteView misc', () async { + final store = eg.store(); + final connection = store.connection as FakeApiConnection; + final first = eg.getStreamTopicsEntry(maxId: 1, name: 'First Topic'); + final second = eg.getStreamTopicsEntry(maxId: 2, name: 'Second Topic'); + final third = eg.getStreamTopicsEntry(maxId: 3, name: 'Third Topic'); + connection.prepare(json: GetStreamTopicsResult( + topics: [first, second, third]).toJson()); + final view = TopicAutocompleteView.init( + store: store, + streamId: eg.stream().streamId); + + bool done = false; + view.addListener(() { done = true; }); + view.query = TopicAutocompleteQuery('Third'); + // those are here to wait for topics to be loaded + await Future(() {}); + await Future(() {}); + check(done).isTrue(); + check(view.results).single + .isA() + .topic.equals(third.name); + }); + + test('TopicAutocompleteView updates results when streams are loaded', () async { + final store = eg.store(); + final connection = store.connection as FakeApiConnection; + connection.prepare(json: GetStreamTopicsResult( + topics: [eg.getStreamTopicsEntry(name: 'test')] + ).toJson()); + + final view = TopicAutocompleteView.init( + store: store, + streamId: eg.stream().streamId); + + bool done = false; + view.addListener(() { done = true; }); + view.query = TopicAutocompleteQuery('te'); + + check(done).isFalse(); + await Future(() {}); + check(done).isTrue(); + }); + + group('TopicAutocompleteQuery.testTopic', () { + void doCheck(String rawQuery, String topic, bool expected) { + final result = TopicAutocompleteQuery(rawQuery).testTopic(topic); + expected ? check(result).isTrue() : check(result).isFalse(); + } + + test('topic is included if it matches the query', () { + doCheck('', 'Top Name', true); + doCheck('Name', 'Name', false); + doCheck('name', 'Name', true); + doCheck('name', 'Nam', false); + doCheck('nam', 'Name', true); + }); + }); } diff --git a/test/widgets/action_sheet_test.dart b/test/widgets/action_sheet_test.dart index 07c9017a69..f2a6403edf 100644 --- a/test/widgets/action_sheet_test.dart +++ b/test/widgets/action_sheet_test.dart @@ -6,6 +6,7 @@ import 'package:flutter/services.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/route/channels.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/binding.dart'; import 'package:zulip/model/compose.dart'; @@ -298,6 +299,12 @@ void main() { final composeBoxController = findComposeBoxController(tester)!; final contentController = composeBoxController.contentController; + // Ensure channel-topics are loaded before testing quote & reply behavior + connection.prepare(body: + jsonEncode(GetStreamTopicsResult(topics: [eg.getStreamTopicsEntry()]).toJson())); + final topicController = composeBoxController.topicController; + topicController?.value = const TextEditingValue(text: kNoTopicTopic); + final valueBefore = contentController.value; prepareRawContentResponseSuccess(store, message: message, rawContent: 'Hello world'); await tapQuoteAndReplyButton(tester); diff --git a/test/widgets/autocomplete_test.dart b/test/widgets/autocomplete_test.dart index 7d4b34724a..15c294696e 100644 --- a/test/widgets/autocomplete_test.dart +++ b/test/widgets/autocomplete_test.dart @@ -3,7 +3,9 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:zulip/api/model/model.dart'; import 'package:zulip/api/route/messages.dart'; +import 'package:zulip/api/route/channels.dart'; import 'package:zulip/model/compose.dart'; +import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; import 'package:zulip/model/store.dart'; import 'package:zulip/widgets/message_list.dart'; @@ -61,6 +63,45 @@ Future setupToComposeInput(WidgetTester tester, { return finder; } +/// Simulates loading a [MessageListPage] with a stream narrow +/// and tapping to focus the topic input. +/// +/// Also prepares test-topics to be sent to topics api request, +/// so they can show up in autocomplete. +/// +/// Returns a [Finder] for the topic input's [TextField]. +Future setupToTopicInput(WidgetTester tester, { + required List topics, +}) async { + addTearDown(testBinding.reset); + await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot()); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + final connection = store.connection as FakeApiConnection; + + // prepare message list data + final stream = eg.stream(); + final message = eg.streamMessage(stream: stream, sender: eg.selfUser); + connection.prepare(json: GetMessagesResult( + anchor: message.id, + foundNewest: true, + foundOldest: true, + foundAnchor: true, + historyLimited: false, + messages: [message], + ).toJson()); + + await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, + child: MessageListPage(initNarrow: ChannelNarrow(stream.streamId)))); + await tester.pumpAndSettle(); + + connection.prepare(json: GetStreamTopicsResult(topics: topics).toJson()); + final zulipLocalizations = GlobalLocalizations.zulipLocalizations; + final finder = find.byWidgetPredicate((widget) => widget is TextField + && widget.decoration?.hintText == zulipLocalizations.composeBoxTopicHintText); + check(finder.evaluate()).isNotEmpty(); + return finder; +} + void main() { TestZulipBinding.ensureInitialized(); @@ -126,4 +167,44 @@ void main() { debugNetworkImageHttpClientProvider = null; }); }); + + group('TopicAutocomplete', () { + void checkTopicShown(GetStreamTopicsEntry topic, PerAccountStore store, {required bool expected}) { + check(find.text(topic.name).evaluate().length).equals(expected ? 1 : 0); + } + + testWidgets('options appear, disappear, and change correctly', (WidgetTester tester) async { + final topic1 = eg.getStreamTopicsEntry(maxId: 1, name: 'Topic one'); + final topic2 = eg.getStreamTopicsEntry(maxId: 2, name: 'Topic two'); + final topic3 = eg.getStreamTopicsEntry(maxId: 3, name: 'Topic three'); + final topicInputFinder = await setupToTopicInput(tester, topics: [topic1, topic2, topic3]); + final store = await testBinding.globalStore.perAccount(eg.selfAccount.id); + + // Options are filtered correctly for query + // TODO(#226): Remove this extra edit when this bug is fixed. + await tester.enterText(topicInputFinder, 'Topic'); + await tester.enterText(topicInputFinder, 'Topic T'); + await tester.pumpAndSettle(); + + // "topic three" and "topic two" appear, but not "topic one" + checkTopicShown(topic1, store, expected: false); + checkTopicShown(topic2, store, expected: true); + checkTopicShown(topic3, store, expected: true); + + // Finishing autocomplete updates topic box; causes options to disappear + await tester.tap(find.text('Topic three')); + await tester.pumpAndSettle(); + check(tester.widget(topicInputFinder).controller!.text) + .equals(topic3.name); + checkTopicShown(topic1, store, expected: false); + checkTopicShown(topic2, store, expected: false); + checkTopicShown(topic3, store, expected: true); // shown in `_TopicInput` once + + // Then a new autocomplete intent brings up options again + await tester.enterText(topicInputFinder, 'Topic'); + await tester.enterText(topicInputFinder, 'Topic T'); + await tester.pumpAndSettle(); + checkTopicShown(topic2, store, expected: true); + }); + }); } diff --git a/test/widgets/compose_box_test.dart b/test/widgets/compose_box_test.dart index 1a1d677bbd..7b9c5017cd 100644 --- a/test/widgets/compose_box_test.dart +++ b/test/widgets/compose_box_test.dart @@ -8,6 +8,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:image_picker/image_picker.dart'; import 'package:zulip/api/model/events.dart'; import 'package:zulip/api/model/model.dart'; +import 'package:zulip/api/route/channels.dart'; import 'package:zulip/api/route/messages.dart'; import 'package:zulip/model/localizations.dart'; import 'package:zulip/model/narrow.dart'; @@ -39,6 +40,11 @@ void main() { await store.addUsers([eg.selfUser, ...users]); connection = store.connection as FakeApiConnection; + if (narrow is ChannelNarrow) { + // Ensure topics are loaded before testing actual logic. + connection.prepare(body: + jsonEncode(GetStreamTopicsResult(topics: [eg.getStreamTopicsEntry()]).toJson())); + } final controllerKey = GlobalKey(); await tester.pumpWidget(TestZulipApp(accountId: eg.selfAccount.id, child: ComposeBox(controllerKey: controllerKey, narrow: narrow)));