make suggestionsBuilder in SearchAnchor asyncable
#127019
Conversation
flutter-dashboard
bot
added
f: material design
github-actions
bot
added
d: api docs
|
I think it will not be a breaking change if using |
|
It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!). If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix? Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
sun-jiao
changed the title
suggestionsBuilder in SearchAnchor asyncsuggestionsBuilder in SearchAnchor asyncable
|
I'm not sure if new test cases are needed. |
There was a problem hiding this comment.
Thanks for doing this! I think you're right that we should be able to do this in an async fashion, it seems like a very common use case. And good call using FutureOr.
This reminds me of AutocompleteOptionsBuilder.
Could you actually add an example to the docs of how to use this with a fake "backend"? I just recently added the same kind of example for Autocomplete because people were confused by it: https://github.com/flutter/flutter/pull/126283/files
| suggestionsFutureOr.then((Iterable<Widget> value) { | ||
| result = value; | ||
| }); |
There was a problem hiding this comment.
What happens if suggestionsFutureOr throws an error?
| void didChangeDependencies() { | ||
| super.didChangeDependencies(); | ||
| result = widget.suggestionsBuilder(context, _controller); | ||
| final FutureOr<Iterable<Widget>> suggestionsFutureOr = widget.suggestionsBuilder(context, _controller); |
There was a problem hiding this comment.
Can you make this async and then do this?
result = await widget.suggestionsBuilder(context, _controller);
There was a problem hiding this comment.
Is it right? didChangeDependencies() overrides a sync function.
There was a problem hiding this comment.
I think it works? The code below compiles for me. Or any reason not to?
class Foo extends StatefulWidget {
const Foo({
super.key,
});
@override
State<Foo> createState() => _FooState();
}
class _FooState extends State<Foo> {
@override
void didUpdateWidget(Foo oldWidget) async {
super.didUpdateWidget(oldWidget);
await Future.delayed(const Duration(seconds: 1));
print('delayed hello');
}
@override
Widget build(BuildContext context) {
return Container(
child: const Text('Foo'),
);
}
}
There was a problem hiding this comment.
Should we also change the function type to Future<void> didChangeDependencies? I tried it locally and the linter shows a warning about its type. This works for me😀:
There was a problem hiding this comment.
Oh, I didn't realize that it could be placed on the last line to avoid using context across async.
I'm trying if (!mounted) {return;} locally.
|
Golden file changes have been found for this pull request. Click here to view and triage (e.g. because this is an intentional change). If you are still iterating on this change and are not ready to resolve the images on the Flutter Gold dashboard, consider marking this PR as a draft pull request above. You will still be able to view image results on the dashboard, commenting will be silenced, and the check will not try to resolve itself until marked ready for review. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
| _viewRect = Offset.zero & _screenSize!; | ||
| } | ||
| } | ||
| result = await widget.suggestionsBuilder(context, _controller); |
There was a problem hiding this comment.
Does this need to be wrapped in a setState since it happens asynchronously?
There was a problem hiding this comment.
I think it is equivalent to calling updateSuggestions().
4 participants
Make
suggestionsBuilderinSearchAnchorasyncable. Because the search results usually come from an async API, for example, a http request.issue:
#126531
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.(Deleted as it is no longer a breaking change.)Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.