AutocompleteCore #62927

justinmc · 2020-08-05T00:41:34Z

Description

This is the first of several PRs implementing autocomplete. This PR only contains AutocompleteCore, the minimal class in the widgets package. The full Autocomplete widget will be added in a subsequent PR.

See the design doc for an exploration of the overall autocomplete plan.

Related Issues

#46824

Tests

I added the following tests:

Autocomplete can filter results...
- By default.
- With a custom filter method.
- With a custom data type.
AutocompleteCore can filter and select...
- Strings
- Custom data types
The results follow the field even when it moves.
Can select by submitting the field.
Custom display strings and custom filter strings work for advanced searching.

Breaking Change

No, all new API.

Example

Here is an example I wrote with an AutocompleteCore inside of a Form. The AutocompleteCore is the last field before the submit button.

https://github.com/justinmc/flutter-autocomplete-examples/blob/master/lib/core_basic_form_page.dart

shihaohong · 2020-08-05T14:31:52Z

packages/flutter/lib/src/widgets/autocomplete.dart

+    this.options,
+    this.search,
+    TextEditingController textEditingController,
+  }) : assert(search != null || options != null, "If a search function isn't specified, Autocomplete will search by string on the given options."),


Maybe add a hint to what the user might want to do to correct their app if they trigger the assert, like:

Suggested change

}) : assert(search != null || options != null, "If a search function isn't specified, Autocomplete will search by string on the given options."),

}) : assert(

search != null || options != null,

"Both `search` and `options` cannot be simultaneously null. "

"If a `search` function isn't specified, Autocomplete will attempt "

"to search by matching the string on the given options."

),

Maybe touch upon how _searchByString might work for non-string options

I've changed how this works a bit, broken this assert into two, and put most of the details in the documentation.

shihaohong · 2020-08-05T14:43:15Z

packages/flutter/lib/src/widgets/autocomplete.dart

+  /// case when querying an external service for search results, for example.
+  ///
+  /// Defaults to a simple string-matching search of [options].
+  final AutocompleteSearchFunction<T> search;


Would AutocompleteFilter and filter be a better names for this function, since it returns a list of filtered options?

This is from my JS background seeing that Array.filter() basically "filters" for values that pass a test, whereas String.search() basically searches for substrings in a string and returns the position of the match. That might not be a very compelling argument, but I figured if the terminology matched another language's, it might feel more "ergonomic" to our users.

Yeah I think you're right. This function may sometimes get more complicated than a simple filter, such as when calling a backend endpoint, but I think that it will always have the ability to return multiple results, which is more like a filter than a search. I'll change the naming.

shihaohong · 2020-08-05T14:44:14Z

packages/flutter/lib/src/widgets/autocomplete.dart

+
+  /// The current results being returned by [search].
+  ///
+  /// This is a [ValueNotifier] so that UI may be updated when results change.


I'm wondering if this might also be used to update the app's state as well (if the app developer somehow wanted to store the results for any reason).

True, I'll reword it to be more generic.

shihaohong · 2020-08-05T14:50:52Z

packages/flutter/lib/src/widgets/autocomplete.dart

+  // the dispose method of the widget it was created in.
+  void dispose() {
+    textEditingController.removeListener(_onQueryChanged);
+    // TODO(justinmc): Shouldn't be disposed if it wasn't created here.


Do we need to provide a dispose method for the AutocompleteController. In the same vein, do we need to remove the listener before disposing of the TextEditingController? If we don't have to explicitly remove listeners before disposing, we could look at animation controller usage as an example and simply advise users to dispose of the TextEditingController directly from their app.

I've worked around this by adding a bool _ownsTextEditingController that is set to true when a TextEditingController isn't passed in. That way I can decide to dispose the TextEditingController only if it was created by the AutocompleteController. Otherwise it's up to the creator that passed it in to dispose it.

shihaohong · 2020-08-05T15:07:45Z

packages/flutter/lib/src/widgets/autocomplete.dart

+  /// Builds the field that is used to input the query.
+  final AutocompleteFieldBuilder buildField;
+
+  /// Builds the selectable results of searching.
+  final AutocompleteResultsBuilder<T> buildResults;


At some point, we may need to provide a very simple example for these two so that users know what they're doing here. Then again, it might not be necessary if we have examples via Autocomplete and AutocompleteCupertino implementations, as long as they're simple enough.

Yeah I agree. I will add a top level example to AutocompleteCore for this PR at least. It might be nice to add them to buildField and buildResults as well.

I've added an example to AutocompleteCore and AutocompleteController, but I didn't add them to the builders since it's covered by the AutocompleteCore example. When we implement the Material Autocomplete we should add an example to buildField and buildResults there, because there they have default values and can be used independently of each other.

shihaohong · 2020-08-05T15:10:36Z

packages/flutter/lib/src/widgets/autocomplete.dart

+  void onSelected (T result) {
+    setState(() {
+      _selection = result;
+      widget.autocompleteController.textEditingController.text = result.toString();


is T.toString()-ing non-string types a normal thing for the TextEditingController? I notice you did that here and with the _searchByString, and I'm wondering how this actually works out in practice.

I'm thinking that T will often be some app-specific type like a User object, and if the developer wants Autocomplete to search their Users automatically, then they need to provide a sensible toString method. If their search logic is even more complicated than that, then they can pass a custom search function.

Actually I just added an example of this to the AutocompleteCore docs because you're right that it's not immediately clear how to do this. Let me know it makes sense to you after looking at the example.

I also ended up adding the onSelected parameter to allow users to customize that. It's useful when using a custom type in case you want to populate the field with something different than toString. It's also in the example.

shihaohong · 2020-08-05T15:14:42Z

packages/flutter/lib/src/widgets/autocomplete.dart

+          widget.autocompleteController.textEditingController,
+        ),
+        if (_selection == null)
+          // TODO(justinmc): should this expanded be here?


I feel like in most cases, this does make sense. I'm trying to imagine an odd layout where a developer would not want the results to be expanded, but can't off the top of my head.

…via closure

…ets yet

…sults sometimes showing after selection

LongCatIsLooong · 2020-10-21T00:26:56Z

packages/flutter/lib/src/widgets/autocomplete.dart

+  // True iff the state indicates that the options should be visible.
+  bool get _shouldShowOptions {
+    final TextSelection selection = _textEditingController.selection;
+    final bool fieldIsFocused = selection.baseOffset >= 0


Yeah sounds good. On desktop people may want to navigate through the options using the keyboard without unfocusing the text field, and unfortunately the text field and the options UI are on different overlay entries, so each option may want to attach to the text field's FocusNode as well.

LongCatIsLooong · 2020-10-21T01:00:28Z

packages/flutter/lib/src/widgets/autocomplete.dart

+    required this.optionsBuilder,
+    this.displayStringForOption = _defaultStringForOption,
+    this.onSelected,
+  }) : assert(fieldViewBuilder != null),


Looks like we still need these for a while? dart-lang/language#1018.

LongCatIsLooong · 2020-10-21T01:25:34Z

packages/flutter/lib/src/widgets/autocomplete.dart

+      _textEditingController.value,
+    );
+    assert(options != null);
+    setState(() {


nit: do we need to rebuild this widget? TextEditingController will rebuild the text field if needed, and the options UI is on a different overlay entry?

Also is the setState in _select needed?

You're right, I was able to remove these by refactoring a bit to make sure updateOverlay is called in the right place. When we implement flat results we may need to add them back, though.

LongCatIsLooong · 2020-10-21T01:32:50Z

packages/flutter/lib/src/widgets/autocomplete.dart

+
+  // Select the given option and update the widget.
+  void _select(T? nextSelection) {
+    if (nextSelection == _selection) {


When should the developer call _select with null?

Also does this mean we want the developer to implement == and hashCode, if they're generating the list of options on the fly?

Calling _select with null: Good point, they shouldn't. I will make it non-nullable. If the user wants to clear the selection, they can just clear or change the field.

About comparison: Yes, I will document that.

… by what is returned from optionsBuilder

justinmc · 2020-10-22T18:11:23Z

@LongCatIsLooong Updated with your suggestions, thanks!

LongCatIsLooong · 2020-10-23T00:07:51Z

packages/flutter/lib/src/widgets/autocomplete.dart

+    }
+    _selection = nextSelection;
+    final String selectionString = nextSelection == null
+        ? ''


nit: nextSelection can not be null?

Yes, thanks!

LongCatIsLooong · 2020-10-23T00:14:32Z

packages/flutter/lib/src/widgets/autocomplete.dart

+class _AutocompleteCoreState<T extends Object> extends State<AutocompleteCore<T>> {
+  final GlobalKey _fieldKey = GlobalKey();
+  final LayerLink _optionsLayerLink = LayerLink();
+  final TextEditingController _textEditingController = TextEditingController();


Should we let the user bring their own controller?

Yes, that is my next PR! I tried to simplify it by keeping it private for now. It will be necessary to use your own controller if you want to do things like split the field and results into different parts of the widget tree. I'll be tagging you for review soon :)

zaidkazi · 2021-03-07T01:24:55Z

How can the optionsBuilder be used to perform async tasks such as retrieving options from a third party search provider like Algolia rather than a data structure locally?

Ideally being able to pass a futureOption builder in async function would be awesome.

As an example

   optionsBuilder: (TextEditingValue textEditingValue) async {
                          if (textEditingValue.text == '') {
                            return const Iterable<AlgoliaObjectSnapshot>.empty();
                          }
                          return await algoliaService.algolia
                              .index("somealgoliaindex")
                              .setHitsPerPage(3)
                              .search(pattern)
                              .getObjects()
                              .hits;
                        },

shihaohong · 2021-03-08T06:14:58Z

Hey @zaidkazi, we have discussed async support in the design doc if you're interested in taking a look. @justinmc has also created a proof-of-concept example that would use FutureOr here, and it's a feature we're planning on working on soon!

zaidkazi · 2021-03-08T06:26:42Z

@shihaohong that's awesome thank you

JayPerfetto · 2021-04-01T18:36:24Z

this feature is fairly useless without the ability to do async calls in it. It was never a challenge to filter a fixed list in memory based on a text fields input.....

justinmc · 2021-04-01T21:23:20Z

I agree that that's a vital feature. I have vague plans for it which you've probably seen in the design doc. I want to make sure we get it right with efficient debouncing and everything. Let me know if you have any ideas.

justinmc added the work in progress; do not review label Aug 5, 2020

justinmc requested a review from shihaohong August 5, 2020 00:41

justinmc self-assigned this Aug 5, 2020

flutter-dashboard bot added the framework flutter/packages/flutter repository. See also f: labels. label Aug 5, 2020

justinmc marked this pull request as draft August 5, 2020 00:41

shihaohong reviewed Aug 5, 2020

View reviewed changes

justinmc added 6 commits August 5, 2020 15:18

Working AutocompleteCore and Controller

2579c90

Comment and fix warnings for AutocompleteController

34d69a7

Fixed warnings in AutocompleteCore

ef77ed7

Basic first test

9de8e6a

WIP

8b87900

Dispose controller only when owned

978289b

justinmc force-pushed the autocomplete-core branch from 473cb1e to 978289b Compare August 5, 2020 22:35

justinmc added 15 commits August 5, 2020 16:15

Examples for AutocompleteCore and AutocompleteController

783291d

Custom T type example

e49252a

onSelected parameter

06a9c33

Simplify parameters, but require users to use autocompletecontroller …

9f13674

…via closure

When selected, put cursor at the end of the field

3c8adc3

Test actual widget behavior for User and String types

80ffa67

Remove todos

76aacb2

Remove extra whitespace

7ac8038

Dartdoc analyze fixes

139fc9f

Rename search to filter

5344266

No simultaneous options and filter. Test and document this.

16a919d

Fix dartdoc analysis error

9c64097

Clarify the need to dispose autocompleteController

db48c3c

Can't link to nonexistent Autocomplete and AutocompleteCupertino widg…

24294bc

…ets yet

Fix onselected needing to set texteditingcontroller value, and fix re…

9760e7f

…sults sometimes showing after selection

googlebot added the cla: yes label Aug 13, 2020

AutocompleteController is optional, can just pass in options directly

99b5bc9

LongCatIsLooong reviewed Oct 21, 2020

View reviewed changes

justinmc force-pushed the autocomplete-core branch from f448646 to 3af596e Compare October 21, 2020 15:48

Remove some unnecessary assertions for null

1c243e0

justinmc force-pushed the autocomplete-core branch from 3af596e to 1c243e0 Compare October 21, 2020 15:51

justinmc added 6 commits October 21, 2020 10:38

Refactor out unnecessary setState calls

8ba4de6

Test that the user can choose whether options are displayed initially…

9ce392e

… by what is returned from optionsBuilder

Cant call select with null

4f66858

Document == and hashValue for custom types

e773c06

Focus node

eac3c96

Analyzer docs example typo fix

dd926ae

justinmc added 2 commits October 22, 2020 15:57

Merge branch 'master' into autocomplete-core

0a21801

AutocompleteCore test non-nullable

cfd9ab5

LongCatIsLooong approved these changes Oct 23, 2020

View reviewed changes

justinmc added 2 commits October 22, 2020 17:43

nextSelection cant be null

16c7948

Merge branch 'master' into autocomplete-core

5ea01f0

justinmc merged commit 291ee94 into flutter:master Oct 27, 2020

justinmc deleted the autocomplete-core branch October 27, 2020 15:59

This was referenced Oct 30, 2020

AutocompleteCore => RawAutocomplete #69419

Merged

AutocompleteController #69618

Closed

pedromassangocode mentioned this pull request Nov 18, 2020

Flutter TextFormField not displaying autofill suggestions #70725

Closed

justinmc mentioned this pull request Jan 11, 2021

Autocomplete (Material) #73753

Merged

justinmc mentioned this pull request Jan 22, 2021

Autocomplete options customizability #74509

Closed

wiradikusuma mentioned this pull request Mar 4, 2021

AutocompleteCore in Flutter 2 AbdulRahmanAlHamali/flutter_typeahead#293

Closed

zsoerenm mentioned this pull request Jun 6, 2021

Alt async suggestions zsoerenm/chips_input#15

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AutocompleteCore #62927

AutocompleteCore #62927

justinmc commented Aug 5, 2020 •

edited

shihaohong Aug 5, 2020

shihaohong Aug 5, 2020

justinmc Aug 6, 2020

shihaohong Aug 5, 2020

justinmc Aug 6, 2020

shihaohong Aug 5, 2020

justinmc Aug 5, 2020

shihaohong Aug 5, 2020

justinmc Aug 5, 2020

shihaohong Aug 5, 2020

justinmc Aug 5, 2020

justinmc Aug 5, 2020

shihaohong Aug 5, 2020

justinmc Aug 6, 2020

justinmc Aug 6, 2020

shihaohong Aug 5, 2020

LongCatIsLooong Oct 21, 2020

LongCatIsLooong Oct 21, 2020

LongCatIsLooong Oct 21, 2020 •

edited

LongCatIsLooong Oct 21, 2020

justinmc Oct 21, 2020

LongCatIsLooong Oct 21, 2020 •

edited

LongCatIsLooong Oct 21, 2020

justinmc Oct 21, 2020

justinmc commented Oct 22, 2020

LongCatIsLooong Oct 23, 2020

justinmc Oct 23, 2020

LongCatIsLooong Oct 23, 2020

justinmc Oct 23, 2020

zaidkazi commented Mar 7, 2021

shihaohong commented Mar 8, 2021

zaidkazi commented Mar 8, 2021

JayPerfetto commented Apr 1, 2021

justinmc commented Apr 1, 2021

AutocompleteCore #62927

AutocompleteCore #62927

Conversation

justinmc commented Aug 5, 2020 • edited

Description

Related Issues

Tests

Breaking Change

Example

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LongCatIsLooong Oct 21, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LongCatIsLooong Oct 21, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

justinmc commented Oct 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zaidkazi commented Mar 7, 2021

shihaohong commented Mar 8, 2021

zaidkazi commented Mar 8, 2021

JayPerfetto commented Apr 1, 2021

justinmc commented Apr 1, 2021

justinmc commented Aug 5, 2020 •

edited

LongCatIsLooong Oct 21, 2020 •

edited

LongCatIsLooong Oct 21, 2020 •

edited