Skip to content

Commit 726ec5a

Browse files
committed
autocomplete [nfc]: Separate ComposeAutocompleteQuery/Result from Mention-etc.
The two concepts have meant the same set of possible values so far, because @-mentions are the only type of autocomplete we've had so far in the compose box's content input. As a result we've taken some shortcuts by conflating them. But as we introduce other types of autocomplete in the content input, like for emoji and #-mentions, we have some places that will need to refer to the more general concept while others refer to the more specific one. So separate them out.
1 parent 13385fe commit 726ec5a

File tree

3 files changed

+84
-31
lines changed

3 files changed

+84
-31
lines changed

lib/model/autocomplete.dart

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import 'narrow.dart';
99
import 'store.dart';
1010

1111
extension ComposeContentAutocomplete on ComposeContentController {
12-
AutocompleteIntent<MentionAutocompleteQuery>? autocompleteIntent() {
12+
AutocompleteIntent<ComposeAutocompleteQuery>? autocompleteIntent() {
1313
if (!selection.isValid || !selection.isNormalized) {
1414
// We don't require [isCollapsed] to be true because we've seen that
1515
// autocorrect and even backspace involve programmatically expanding the
@@ -208,6 +208,10 @@ abstract class AutocompleteView<QueryT extends AutocompleteQuery, ResultT extend
208208

209209
final PerAccountStore store;
210210

211+
/// True just if this [AutocompleteView] is of the appropriate type
212+
/// to handle the given query.
213+
bool acceptsQuery(AutocompleteQuery query) => query is QueryT;
214+
211215
/// The last query this [AutocompleteView] was asked to perform for the user.
212216
///
213217
/// If this view-model is currently performing a search,
@@ -311,7 +315,12 @@ abstract class AutocompleteView<QueryT extends AutocompleteQuery, ResultT extend
311315
}
312316
}
313317

314-
/// An [AutocompleteView] for an @-mention autocomplete interaction.
318+
/// An [AutocompleteView] for an autocomplete interaction
319+
/// in the compose box's content input.
320+
typedef ComposeAutocompleteView = AutocompleteView<ComposeAutocompleteQuery, ComposeAutocompleteResult>;
321+
322+
/// An [AutocompleteView] for an @-mention autocomplete interaction,
323+
/// an example of a [ComposeAutocompleteView].
315324
class MentionAutocompleteView extends AutocompleteView<MentionAutocompleteQuery, MentionAutocompleteResult> {
316325
MentionAutocompleteView._({
317326
required super.store,
@@ -572,13 +581,27 @@ abstract class AutocompleteQuery {
572581
}
573582
}
574583

584+
/// Any autocomplete query in the compose box's content input.
585+
abstract class ComposeAutocompleteQuery extends AutocompleteQuery {
586+
ComposeAutocompleteQuery(super.raw);
587+
588+
/// Construct an [AutocompleteView] initialized with this query
589+
/// and ready to handle queries of the same type.
590+
ComposeAutocompleteView initViewModel(PerAccountStore store, Narrow narrow);
591+
}
592+
575593
/// A @-mention autocomplete query, used by [MentionAutocompleteView].
576-
class MentionAutocompleteQuery extends AutocompleteQuery {
594+
class MentionAutocompleteQuery extends ComposeAutocompleteQuery {
577595
MentionAutocompleteQuery(super.raw, {this.silent = false});
578596

579597
/// Whether the user wants a silent mention (@_query, vs. @query).
580598
final bool silent;
581599

600+
@override
601+
MentionAutocompleteView initViewModel(PerAccountStore store, Narrow narrow) {
602+
return MentionAutocompleteView.init(store: store, narrow: narrow, query: this);
603+
}
604+
582605
bool testUser(User user, AutocompleteDataCache cache) {
583606
// TODO(#236) test email too, not just name
584607

@@ -636,13 +659,18 @@ class AutocompleteDataCache {
636659
/// have corresponding subclasses of [AutocompleteResult] they might produce.
637660
class AutocompleteResult {}
638661

662+
/// A result from some autocomplete interaction in
663+
/// the compose box's content input, initiated by a [ComposeAutocompleteQuery]
664+
/// and managed by some [ComposeAutocompleteView].
665+
sealed class ComposeAutocompleteResult extends AutocompleteResult {}
666+
639667
/// A result from an @-mention autocomplete interaction,
640668
/// managed by a [MentionAutocompleteView].
641669
///
642670
/// This is abstract because there are several kinds of result
643671
/// that can all be offered in the same @-mention autocomplete interaction:
644672
/// a user, a wildcard, or a user group.
645-
sealed class MentionAutocompleteResult extends AutocompleteResult {}
673+
sealed class MentionAutocompleteResult extends ComposeAutocompleteResult {}
646674

647675
/// An autocomplete result for an @-mention of an individual user.
648676
class UserMentionAutocompleteResult extends MentionAutocompleteResult {

lib/widgets/autocomplete.dart

Lines changed: 51 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,23 @@ class _AutocompleteFieldState<QueryT extends AutocompleteQuery, ResultT extends
3939

4040
void _handleControllerChange() {
4141
final newQuery = widget.autocompleteIntent()?.query;
42+
// First, tear down the old view-model if necessary.
43+
if (_viewModel != null
44+
&& (newQuery == null
45+
|| !_viewModel!.acceptsQuery(newQuery))) {
46+
// The autocomplete interaction has ended, or has switched to a
47+
// different kind of autocomplete (e.g. @-mention vs. emoji).
48+
_viewModel!.dispose(); // removes our listener
49+
_viewModel = null;
50+
_resultsToDisplay = [];
51+
}
52+
// Then, update the view-model or build a new one.
4253
if (newQuery != null) {
4354
if (_viewModel == null) {
4455
_initViewModel(newQuery);
45-
}
46-
_viewModel!.query = newQuery;
47-
} else {
48-
if (_viewModel != null) {
49-
_viewModel!.dispose(); // removes our listener
50-
_viewModel = null;
51-
_resultsToDisplay = [];
56+
} else {
57+
assert(_viewModel!.acceptsQuery(newQuery));
58+
_viewModel!.query = newQuery;
5259
}
5360
}
5461
}
@@ -144,7 +151,7 @@ class _AutocompleteFieldState<QueryT extends AutocompleteQuery, ResultT extends
144151
}
145152
}
146153

147-
class ComposeAutocomplete extends AutocompleteField<MentionAutocompleteQuery, MentionAutocompleteResult> {
154+
class ComposeAutocomplete extends AutocompleteField<ComposeAutocompleteQuery, ComposeAutocompleteResult> {
148155
const ComposeAutocomplete({
149156
super.key,
150157
required this.narrow,
@@ -159,30 +166,34 @@ class ComposeAutocomplete extends AutocompleteField<MentionAutocompleteQuery, Me
159166
ComposeContentController get controller => super.controller as ComposeContentController;
160167

161168
@override
162-
AutocompleteIntent<MentionAutocompleteQuery>? autocompleteIntent() => controller.autocompleteIntent();
169+
AutocompleteIntent<ComposeAutocompleteQuery>? autocompleteIntent() => controller.autocompleteIntent();
163170

164171
@override
165-
MentionAutocompleteView initViewModel(BuildContext context, MentionAutocompleteQuery query) {
172+
ComposeAutocompleteView initViewModel(BuildContext context, ComposeAutocompleteQuery query) {
166173
final store = PerAccountStoreWidget.of(context);
167-
return MentionAutocompleteView.init(store: store, narrow: narrow, query: query);
174+
return query.initViewModel(store, narrow);
168175
}
169176

170-
void _onTapOption(BuildContext context, MentionAutocompleteResult option) {
177+
void _onTapOption(BuildContext context, ComposeAutocompleteResult option) {
171178
// Probably the same intent that brought up the option that was tapped.
172179
// If not, it still shouldn't be off by more than the time it takes
173180
// to compute the autocomplete results, which we do asynchronously.
174181
final intent = autocompleteIntent();
175182
if (intent == null) {
176183
return; // Shrug.
177184
}
185+
final query = intent.query;
178186

179187
final store = PerAccountStoreWidget.of(context);
180188
final String replacementString;
181189
switch (option) {
182190
case UserMentionAutocompleteResult(:var userId):
191+
if (query is! MentionAutocompleteQuery) {
192+
return; // Shrug; similar to `intent == null` case above.
193+
}
183194
// TODO(i18n) language-appropriate space character; check active keyboard?
184195
// (maybe handle centrally in `controller`)
185-
replacementString = '${mention(store.users[userId]!, silent: intent.query.silent, users: store.users)} ';
196+
replacementString = '${mention(store.users[userId]!, silent: query.silent, users: store.users)} ';
186197
}
187198

188199
controller.value = intent.textEditingValue.replaced(
@@ -194,26 +205,40 @@ class ComposeAutocomplete extends AutocompleteField<MentionAutocompleteQuery, Me
194205
}
195206

196207
@override
197-
Widget buildItem(BuildContext context, int index, MentionAutocompleteResult option) {
208+
Widget buildItem(BuildContext context, int index, ComposeAutocompleteResult option) {
209+
final child = switch (option) {
210+
MentionAutocompleteResult() => _MentionAutocompleteItem(option: option),
211+
};
212+
return InkWell(
213+
onTap: () {
214+
_onTapOption(context, option);
215+
},
216+
child: child);
217+
}
218+
}
219+
220+
class _MentionAutocompleteItem extends StatelessWidget {
221+
const _MentionAutocompleteItem({required this.option});
222+
223+
final MentionAutocompleteResult option;
224+
225+
@override
226+
Widget build(BuildContext context) {
198227
Widget avatar;
199228
String label;
200229
switch (option) {
201230
case UserMentionAutocompleteResult(:var userId):
202231
avatar = Avatar(userId: userId, size: 32, borderRadius: 3);
203232
label = PerAccountStoreWidget.of(context).users[userId]!.fullName;
204233
}
205-
return InkWell(
206-
onTap: () {
207-
_onTapOption(context, option);
208-
},
209-
child: Padding(
210-
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
211-
child: Row(
212-
children: [
213-
avatar,
214-
const SizedBox(width: 8),
215-
Text(label),
216-
])));
234+
235+
return Padding(
236+
padding: const EdgeInsets.symmetric(horizontal: 16.0, vertical: 8.0),
237+
child: Row(children: [
238+
avatar,
239+
const SizedBox(width: 8),
240+
Text(label),
241+
]));
217242
}
218243
}
219244

test/model/autocomplete_checks.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import 'package:zulip/model/autocomplete.dart';
33
import 'package:zulip/widgets/compose_box.dart';
44

55
extension ComposeContentControllerChecks on Subject<ComposeContentController> {
6-
Subject<AutocompleteIntent<MentionAutocompleteQuery>?> get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent');
6+
Subject<AutocompleteIntent<ComposeAutocompleteQuery>?> get autocompleteIntent => has((c) => c.autocompleteIntent(), 'autocompleteIntent');
77
}
88

99
extension ComposeTopicControllerChecks on Subject<ComposeTopicController> {

0 commit comments

Comments
 (0)