Skip to content

Commit 777245f

Browse files
committed
autocomplete [nfc]: Make query non-nullable on AutocompleteView
At first glance this appears to make a functional change: it causes the search to begin as soon as the AutocompleteView is constructed, rather than later when `query` is set to non-null. But in fact the only times (outside tests) that we ultimately construct an AutocompleteView are by calling an implementation of AutocompleteField.initViewModel... and those call sites are immediately followed by setting a non-null query. So this is an NFC commit after all.
1 parent e561f3a commit 777245f

File tree

3 files changed

+77
-58
lines changed

3 files changed

+77
-58
lines changed

lib/model/autocomplete.dart

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -184,37 +184,42 @@ class AutocompleteViewManager {
184184
/// and the [AutocompleteResult] subclass in `ResultT` describes the
185185
/// possible results that the user might choose in the autocomplete interaction.
186186
///
187+
/// When an [AutocompleteView] is created, its constructor begins the search
188+
/// for results corresponding to the initial [query].
189+
/// The query may later be updated, causing a new search.
190+
///
187191
/// The owner of one of these objects must call [dispose] when the object
188192
/// will no longer be used, in order to free resources on the [PerAccountStore].
189193
///
190194
/// Lifecycle:
191-
/// * Create an instance of a concrete subtype.
195+
/// * Create an instance of a concrete subtype, beginning a search
192196
/// * Add listeners with [addListener].
193-
/// * Use the [query] setter to start a search for a query.
197+
/// * When the user edits the query, use the [query] setter to update the search.
194198
/// * On reassemble, call [reassemble].
195199
/// * When the object will no longer be used, call [dispose] to free
196200
/// resources on the [PerAccountStore].
197201
abstract class AutocompleteView<QueryT extends AutocompleteQuery, ResultT extends AutocompleteResult> extends ChangeNotifier {
198-
AutocompleteView({required this.store});
202+
/// Construct a view-model for an autocomplete interaction,
203+
/// and begin the search for the initial query.
204+
AutocompleteView({required this.store, required QueryT query})
205+
: _query = query {
206+
_startSearch();
207+
}
199208

200209
final PerAccountStore store;
201210

202-
QueryT? get query => _query;
203-
QueryT? _query;
204-
set query(QueryT? query) {
211+
QueryT get query => _query;
212+
QueryT _query;
213+
set query(QueryT query) {
205214
_query = query;
206-
if (query != null) {
207-
_startSearch();
208-
}
215+
_startSearch();
209216
}
210217

211218
/// Called when the app is reassembled during debugging, e.g. for hot reload.
212219
///
213220
/// This will redo the search from scratch for the current query, if any.
214221
void reassemble() {
215-
if (_query != null) {
216-
_startSearch();
217-
}
222+
_startSearch();
218223
}
219224

220225
Iterable<ResultT> get results => _results;
@@ -271,8 +276,7 @@ abstract class AutocompleteView<QueryT extends AutocompleteQuery, ResultT extend
271276
required Iterable<T> candidates,
272277
required List<ResultT> results,
273278
}) async {
274-
assert(_query != null);
275-
final query = _query!;
279+
final query = _query;
276280

277281
final iterator = candidates.iterator;
278282
outer: while (true) {
@@ -295,16 +299,19 @@ abstract class AutocompleteView<QueryT extends AutocompleteQuery, ResultT extend
295299
class MentionAutocompleteView extends AutocompleteView<MentionAutocompleteQuery, MentionAutocompleteResult> {
296300
MentionAutocompleteView._({
297301
required super.store,
302+
required super.query,
298303
required this.narrow,
299304
required this.sortedUsers,
300305
});
301306

302307
factory MentionAutocompleteView.init({
303308
required PerAccountStore store,
304309
required Narrow narrow,
310+
required MentionAutocompleteQuery query,
305311
}) {
306312
final view = MentionAutocompleteView._(
307313
store: store,
314+
query: query,
308315
narrow: narrow,
309316
sortedUsers: _usersByRelevance(store: store, narrow: narrow),
310317
);
@@ -634,10 +641,19 @@ class UserMentionAutocompleteResult extends MentionAutocompleteResult {
634641

635642
/// An autocomplete interaction for choosing a topic for a message.
636643
class TopicAutocompleteView extends AutocompleteView<TopicAutocompleteQuery, TopicAutocompleteResult> {
637-
TopicAutocompleteView._({required super.store, required this.streamId});
644+
TopicAutocompleteView._({
645+
required super.store,
646+
required super.query,
647+
required this.streamId,
648+
});
638649

639-
factory TopicAutocompleteView.init({required PerAccountStore store, required int streamId}) {
640-
final view = TopicAutocompleteView._(store: store, streamId: streamId);
650+
factory TopicAutocompleteView.init({
651+
required PerAccountStore store,
652+
required int streamId,
653+
required TopicAutocompleteQuery query,
654+
}) {
655+
final view = TopicAutocompleteView._(
656+
store: store, streamId: streamId, query: query);
641657
store.autocompleteViewManager.registerTopicAutocomplete(view);
642658
view._fetch();
643659
return view;
@@ -661,7 +677,7 @@ class TopicAutocompleteView extends AutocompleteView<TopicAutocompleteQuery, Top
661677
final result = await getStreamTopics(store.connection, streamId: streamId);
662678
_topics = result.topics.map((e) => e.name);
663679
_isFetching = false;
664-
if (_query != null) return _startSearch();
680+
return _startSearch();
665681
}
666682

667683
@override

lib/widgets/autocomplete.dart

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ abstract class AutocompleteField<QueryT extends AutocompleteQuery, ResultT exten
2323

2424
Widget buildItem(BuildContext context, int index, ResultT option);
2525

26-
AutocompleteView<QueryT, ResultT> initViewModel(BuildContext context);
26+
AutocompleteView<QueryT, ResultT> initViewModel(BuildContext context, QueryT query);
2727

2828
@override
2929
State<AutocompleteField<QueryT, ResultT>> createState() => _AutocompleteFieldState<QueryT, ResultT>();
@@ -32,18 +32,18 @@ abstract class AutocompleteField<QueryT extends AutocompleteQuery, ResultT exten
3232
class _AutocompleteFieldState<QueryT extends AutocompleteQuery, ResultT extends AutocompleteResult> extends State<AutocompleteField<QueryT, ResultT>> with PerAccountStoreAwareStateMixin<AutocompleteField<QueryT, ResultT>> {
3333
AutocompleteView<QueryT, ResultT>? _viewModel;
3434

35-
void _initViewModel() {
36-
_viewModel = widget.initViewModel(context)
35+
void _initViewModel(QueryT query) {
36+
_viewModel = widget.initViewModel(context, query)
3737
..addListener(_viewModelChanged);
3838
}
3939

4040
void _handleControllerChange() {
41-
final newAutocompleteIntent = widget.autocompleteIntent();
42-
if (newAutocompleteIntent != null) {
41+
final newQuery = widget.autocompleteIntent()?.query;
42+
if (newQuery != null) {
4343
if (_viewModel == null) {
44-
_initViewModel();
44+
_initViewModel(newQuery);
4545
}
46-
_viewModel!.query = newAutocompleteIntent.query;
46+
_viewModel!.query = newQuery;
4747
} else {
4848
if (_viewModel != null) {
4949
_viewModel!.dispose(); // removes our listener
@@ -64,7 +64,7 @@ class _AutocompleteFieldState<QueryT extends AutocompleteQuery, ResultT extends
6464
if (_viewModel != null) {
6565
final query = _viewModel!.query;
6666
_viewModel!.dispose();
67-
_initViewModel();
67+
_initViewModel(query);
6868
_viewModel!.query = query;
6969
}
7070
}
@@ -162,9 +162,9 @@ class ComposeAutocomplete extends AutocompleteField<MentionAutocompleteQuery, Me
162162
AutocompleteIntent<MentionAutocompleteQuery>? autocompleteIntent() => controller.autocompleteIntent();
163163

164164
@override
165-
MentionAutocompleteView initViewModel(BuildContext context) {
165+
MentionAutocompleteView initViewModel(BuildContext context, MentionAutocompleteQuery query) {
166166
final store = PerAccountStoreWidget.of(context);
167-
return MentionAutocompleteView.init(store: store, narrow: narrow);
167+
return MentionAutocompleteView.init(store: store, narrow: narrow, query: query);
168168
}
169169

170170
void _onTapOption(BuildContext context, MentionAutocompleteResult option) {
@@ -238,9 +238,9 @@ class TopicAutocomplete extends AutocompleteField<TopicAutocompleteQuery, TopicA
238238
AutocompleteIntent<TopicAutocompleteQuery>? autocompleteIntent() => controller.autocompleteIntent();
239239

240240
@override
241-
TopicAutocompleteView initViewModel(BuildContext context) {
241+
TopicAutocompleteView initViewModel(BuildContext context, TopicAutocompleteQuery query) {
242242
final store = PerAccountStoreWidget.of(context);
243-
return TopicAutocompleteView.init(store: store, streamId: streamId);
243+
return TopicAutocompleteView.init(store: store, streamId: streamId, query: query);
244244
}
245245

246246
void _onTapOption(BuildContext context, TopicAutocompleteResult option) {

test/model/autocomplete_test.dart

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -175,11 +175,11 @@ void main() {
175175
const narrow = ChannelNarrow(1);
176176
final store = eg.store();
177177
await store.addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]);
178-
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
179178

179+
final view = MentionAutocompleteView.init(store: store, narrow: narrow,
180+
query: MentionAutocompleteQuery('Third'));
180181
bool done = false;
181182
view.addListener(() { done = true; });
182-
view.query = MentionAutocompleteQuery('Third');
183183
await Future(() {});
184184
await Future(() {});
185185
check(done).isTrue();
@@ -193,12 +193,8 @@ void main() {
193193
const narrow = ChannelNarrow(1);
194194
final store = eg.store();
195195
await store.addUsers([eg.selfUser, eg.otherUser, eg.thirdUser]);
196-
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
197196

198197
bool searchDone = false;
199-
view.addListener(() {
200-
searchDone = true;
201-
});
202198

203199
// Schedule a timer event with zero delay.
204200
// This stands in for a user interaction, or frame rendering timer,
@@ -210,9 +206,11 @@ void main() {
210206
check(searchDone).isFalse();
211207
});
212208

213-
view.query = MentionAutocompleteQuery('Third');
214-
check(timerDone).isFalse();
215-
check(searchDone).isFalse();
209+
final view = MentionAutocompleteView.init(store: store, narrow: narrow,
210+
query: MentionAutocompleteQuery('Third'));
211+
view.addListener(() {
212+
searchDone = true;
213+
});
216214

217215
binding.elapse(const Duration(seconds: 1));
218216

@@ -230,11 +228,11 @@ void main() {
230228
for (int i = 1; i <= 2500; i++) {
231229
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
232230
}
233-
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
234231

235232
bool done = false;
233+
final view = MentionAutocompleteView.init(store: store, narrow: narrow,
234+
query: MentionAutocompleteQuery('User 2222'));
236235
view.addListener(() { done = true; });
237-
view.query = MentionAutocompleteQuery('User 2222');
238236

239237
await Future(() {});
240238
check(done).isFalse();
@@ -253,11 +251,11 @@ void main() {
253251
for (int i = 1; i <= 1500; i++) {
254252
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
255253
}
256-
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
257254

258255
bool done = false;
256+
final view = MentionAutocompleteView.init(store: store, narrow: narrow,
257+
query: MentionAutocompleteQuery('User 1111'));
259258
view.addListener(() { done = true; });
260-
view.query = MentionAutocompleteQuery('User 1111');
261259

262260
await Future(() {});
263261
check(done).isFalse();
@@ -288,11 +286,11 @@ void main() {
288286
for (int i = 1; i <= 2500; i++) {
289287
await store.addUser(eg.user(userId: i, email: 'user$i@example.com', fullName: 'User $i'));
290288
}
291-
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
292289

293290
bool done = false;
291+
final view = MentionAutocompleteView.init(store: store, narrow: narrow,
292+
query: MentionAutocompleteQuery('User 110'));
294293
view.addListener(() { done = true; });
295-
view.query = MentionAutocompleteQuery('User 110');
296294

297295
await Future(() {});
298296
check(done).isFalse();
@@ -544,15 +542,17 @@ void main() {
544542

545543
group('ranking across signals', () {
546544
void checkPrecedes(Narrow narrow, User userA, Iterable<User> usersB) {
547-
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
545+
final view = MentionAutocompleteView.init(store: store, narrow: narrow,
546+
query: MentionAutocompleteQuery(''));
548547
for (final userB in usersB) {
549548
check(view.debugCompareUsers(userA, userB)).isLessThan(0);
550549
check(view.debugCompareUsers(userB, userA)).isGreaterThan(0);
551550
}
552551
}
553552

554553
void checkRankEqual(Narrow narrow, List<User> users) {
555-
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
554+
final view = MentionAutocompleteView.init(store: store, narrow: narrow,
555+
query: MentionAutocompleteQuery(''));
556556
for (int i = 0; i < users.length; i++) {
557557
for (int j = i + 1; j < users.length; j++) {
558558
check(view.debugCompareUsers(users[i], users[j])).equals(0);
@@ -669,21 +669,24 @@ void main() {
669669
test('CombinedFeedNarrow gives error', () async {
670670
await prepare(users: [eg.user(), eg.user()], messages: []);
671671
const narrow = CombinedFeedNarrow();
672-
check(() => MentionAutocompleteView.init(store: store, narrow: narrow))
672+
check(() => MentionAutocompleteView.init(store: store, narrow: narrow,
673+
query: MentionAutocompleteQuery('')))
673674
.throws<AssertionError>();
674675
});
675676

676677
test('MentionsNarrow gives error', () async {
677678
await prepare(users: [eg.user(), eg.user()], messages: []);
678679
const narrow = MentionsNarrow();
679-
check(() => MentionAutocompleteView.init(store: store, narrow: narrow))
680+
check(() => MentionAutocompleteView.init(store: store, narrow: narrow,
681+
query: MentionAutocompleteQuery('')))
680682
.throws<AssertionError>();
681683
});
682684

683685
test('StarredMessagesNarrow gives error', () async {
684686
await prepare(users: [eg.user(), eg.user()], messages: []);
685687
const narrow = StarredMessagesNarrow();
686-
check(() => MentionAutocompleteView.init(store: store, narrow: narrow))
688+
check(() => MentionAutocompleteView.init(store: store, narrow: narrow,
689+
query: MentionAutocompleteQuery('')))
687690
.throws<AssertionError>();
688691
});
689692
});
@@ -692,9 +695,9 @@ void main() {
692695
Future<Iterable<int>> getResults(
693696
Narrow narrow, MentionAutocompleteQuery query) async {
694697
bool done = false;
695-
final view = MentionAutocompleteView.init(store: store, narrow: narrow);
698+
final view = MentionAutocompleteView.init(store: store, narrow: narrow,
699+
query: query);
696700
view.addListener(() { done = true; });
697-
view.query = query;
698701
await Future(() {});
699702
check(done).isTrue();
700703
final results = view.results
@@ -777,13 +780,14 @@ void main() {
777780
final third = eg.getStreamTopicsEntry(maxId: 3, name: 'Third Topic');
778781
connection.prepare(json: GetStreamTopicsResult(
779782
topics: [first, second, third]).toJson());
783+
780784
final view = TopicAutocompleteView.init(
781785
store: store,
782-
streamId: eg.stream().streamId);
783-
786+
streamId: eg.stream().streamId,
787+
query: TopicAutocompleteQuery('Third'));
784788
bool done = false;
785789
view.addListener(() { done = true; });
786-
view.query = TopicAutocompleteQuery('Third');
790+
787791
// those are here to wait for topics to be loaded
788792
await Future(() {});
789793
await Future(() {});
@@ -802,11 +806,10 @@ void main() {
802806

803807
final view = TopicAutocompleteView.init(
804808
store: store,
805-
streamId: eg.stream().streamId);
806-
809+
streamId: eg.stream().streamId,
810+
query: TopicAutocompleteQuery('te'));
807811
bool done = false;
808812
view.addListener(() { done = true; });
809-
view.query = TopicAutocompleteQuery('te');
810813

811814
check(done).isFalse();
812815
await Future(() {});

0 commit comments

Comments
 (0)