-
Notifications
You must be signed in to change notification settings - Fork 28.5k
Added filter callback on dropdown menu #143939
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added filter callback on dropdown menu #143939
Conversation
8aa47ee
to
4f34899
Compare
9b9a75c
to
3ed4b29
Compare
6201d0d
to
d61b4e1
Compare
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. 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. |
d61b4e1
to
4e6c9b0
Compare
@QuncCccccc any idea when you can look over this PR? |
Hi! Sorry for the late response! Overall this is a reasonable feature, will take a look very soon! |
fd6e724
to
7c621a4
Compare
7c621a4
to
6fe0afd
Compare
e16bd71
to
d6fd5aa
Compare
@QuncCccccc can you take one more look here, please? |
d6fd5aa
to
d0e6777
Compare
@QuncCccccc any updates? |
d0e6777
to
815379e
Compare
@QuncCccccc are you still around? @HansMuller Should I contact someone else? |
815379e
to
eef9a56
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your patience! Just left some comments below. After fixing these, we should good to go!
eef9a56
to
ec9eec8
Compare
I think we can just keep it as-is for now, but at the same time we might want to add an assertion to make sure when |
@QuncCccccc I added it. Wasn't sure if I needed to add a comment there or not. |
51fb0b2
to
4632fe9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with some suggestions below.
Just noticed I didn't click Approve last time. I will ask the second review when the comments are fixed:)! Thanks again for the contribution!
No worries @QuncCccccc ! I fixed the comments! Thank you too for helping me getting it done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dacianf I wonder what you think about this comment about possibly setting a default value for filterCallback: https://github.com/flutter/flutter/pull/143939/files#r1621190671
Otherwise everything else is nits and this PR looks good.
remove redundant code revert changes on `_enableFilter` variable fixes based on PR comments Update packages/flutter/test/material/dropdown_menu_test.dart
Co-authored-by: Qun Cheng <[email protected]>
ceb5087
to
32c5675
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍 . Thanks for answering all of my questions and making those fixes!
@@ -162,6 +168,7 @@ class DropdownMenu<T> extends StatefulWidget { | |||
this.focusNode, | |||
this.requestFocusOnTap, | |||
this.expandedInsets, | |||
this.filterCallback, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok agreed, it makes sense to do this the same way as searchCallback for now.
…pdate * master: (168 commits) Fix the scrolling layout deviation of `CupertinoActionSheet` (flutter#149439) Roll Flutter Engine from 60a7bb2353b6 to a6aa5d826649 (2 revisions) (flutter#149627) Roll Flutter Engine from ea72558be758 to 60a7bb2353b6 (2 revisions) (flutter#149623) Place `flutter_gpu` in the package cache. (flutter#149299) Switch to triage-* labels for platform package triage (flutter#149614) Roll pub packages (flutter#149617) Fixes multi line textfield hint text gets ellipsized (flutter#148423) Support failures-only and silent reporters in `flutter test` (flutter#148739) [CupertinoActionSheet] Fix overflow of the overscroll section when the user scrolls far (flutter#149542) Fix InputDecorator.prefixIcon color when disabled (flutter#149595) Added filter callback on dropdown menu (flutter#143939) update generated localized message files in the stocks test app (flutter#148741) Add a simplified SimpleCascadingMenuApp example (flutter#149147) Reland "Prevent LayoutBuilder from rebuilding more than once (flutter#147856)" (flutter#149303) Move some benchmarks from MotoG4 to Mokey (flutter#149567) Roll Packages from d8e8e8c to 11e192a (2 revisions) (flutter#149596) Cleanup triage reports from docs/ (flutter#149545) Roll Flutter Engine from d81edf635a9f to ea72558be758 (1 revision) (flutter#149590) Roll Flutter Engine from b0f4d7459708 to d81edf635a9f (1 revision) (flutter#149468) Roll Flutter Engine from 40b868efcc46 to b0f4d7459708 (1 revision) (flutter#149467) ...
…pdate * master: (168 commits) Fix the scrolling layout deviation of `CupertinoActionSheet` (flutter#149439) Roll Flutter Engine from 60a7bb2353b6 to a6aa5d826649 (2 revisions) (flutter#149627) Roll Flutter Engine from ea72558be758 to 60a7bb2353b6 (2 revisions) (flutter#149623) Place `flutter_gpu` in the package cache. (flutter#149299) Switch to triage-* labels for platform package triage (flutter#149614) Roll pub packages (flutter#149617) Fixes multi line textfield hint text gets ellipsized (flutter#148423) Support failures-only and silent reporters in `flutter test` (flutter#148739) [CupertinoActionSheet] Fix overflow of the overscroll section when the user scrolls far (flutter#149542) Fix InputDecorator.prefixIcon color when disabled (flutter#149595) Added filter callback on dropdown menu (flutter#143939) update generated localized message files in the stocks test app (flutter#148741) Add a simplified SimpleCascadingMenuApp example (flutter#149147) Reland "Prevent LayoutBuilder from rebuilding more than once (flutter#147856)" (flutter#149303) Move some benchmarks from MotoG4 to Mokey (flutter#149567) Roll Packages from d8e8e8c to 11e192a (2 revisions) (flutter#149596) Cleanup triage reports from docs/ (flutter#149545) Roll Flutter Engine from d81edf635a9f to ea72558be758 (1 revision) (flutter#149590) Roll Flutter Engine from b0f4d7459708 to d81edf635a9f (1 revision) (flutter#149468) Roll Flutter Engine from 40b868efcc46 to b0f4d7459708 (1 revision) (flutter#149467) ...
…pdate * master: (181 commits) Fix the scrolling layout deviation of `CupertinoActionSheet` (flutter#149439) Roll Flutter Engine from 60a7bb2353b6 to a6aa5d826649 (2 revisions) (flutter#149627) Roll Flutter Engine from ea72558be758 to 60a7bb2353b6 (2 revisions) (flutter#149623) Place `flutter_gpu` in the package cache. (flutter#149299) Switch to triage-* labels for platform package triage (flutter#149614) Roll pub packages (flutter#149617) Fixes multi line textfield hint text gets ellipsized (flutter#148423) Support failures-only and silent reporters in `flutter test` (flutter#148739) [CupertinoActionSheet] Fix overflow of the overscroll section when the user scrolls far (flutter#149542) Fix InputDecorator.prefixIcon color when disabled (flutter#149595) Added filter callback on dropdown menu (flutter#143939) update generated localized message files in the stocks test app (flutter#148741) Add a simplified SimpleCascadingMenuApp example (flutter#149147) Reland "Prevent LayoutBuilder from rebuilding more than once (flutter#147856)" (flutter#149303) Move some benchmarks from MotoG4 to Mokey (flutter#149567) Roll Packages from d8e8e8c to 11e192a (2 revisions) (flutter#149596) Cleanup triage reports from docs/ (flutter#149545) Roll Flutter Engine from d81edf635a9f to ea72558be758 (1 revision) (flutter#149590) Roll Flutter Engine from b0f4d7459708 to d81edf635a9f (1 revision) (flutter#149468) Roll Flutter Engine from 40b868efcc46 to b0f4d7459708 (1 revision) (flutter#149467) ...
* master: (181 commits) Fix the scrolling layout deviation of `CupertinoActionSheet` (flutter#149439) Roll Flutter Engine from 60a7bb2353b6 to a6aa5d826649 (2 revisions) (flutter#149627) Roll Flutter Engine from ea72558be758 to 60a7bb2353b6 (2 revisions) (flutter#149623) Place `flutter_gpu` in the package cache. (flutter#149299) Switch to triage-* labels for platform package triage (flutter#149614) Roll pub packages (flutter#149617) Fixes multi line textfield hint text gets ellipsized (flutter#148423) Support failures-only and silent reporters in `flutter test` (flutter#148739) [CupertinoActionSheet] Fix overflow of the overscroll section when the user scrolls far (flutter#149542) Fix InputDecorator.prefixIcon color when disabled (flutter#149595) Added filter callback on dropdown menu (flutter#143939) update generated localized message files in the stocks test app (flutter#148741) Add a simplified SimpleCascadingMenuApp example (flutter#149147) Reland "Prevent LayoutBuilder from rebuilding more than once (flutter#147856)" (flutter#149303) Move some benchmarks from MotoG4 to Mokey (flutter#149567) Roll Packages from d8e8e8c to 11e192a (2 revisions) (flutter#149596) Cleanup triage reports from docs/ (flutter#149545) Roll Flutter Engine from d81edf635a9f to ea72558be758 (1 revision) (flutter#149590) Roll Flutter Engine from b0f4d7459708 to d81edf635a9f (1 revision) (flutter#149468) Roll Flutter Engine from 40b868efcc46 to b0f4d7459708 (1 revision) (flutter#149467) ...
I added a callback on DropDown Menu for filtering options. I needed this feature because I wanted to filtered data using a custom way and that was not possible. For searching there was already a callback that helped, but none for filtering.
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.