-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
fix DropdownMenu
keyboard navigation when entries are filtered empty
#155252
fix DropdownMenu
keyboard navigation when entries are filtered empty
#155252
Conversation
add setter to `filteredEntries` to update relevant properties.
DropdownMenu
keyboard navigation when entires are filtered emptyDropdownMenu
keyboard navigation when entries are filtered empty
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.
Many thanks for this fix 🙏 and sorry for the delay.
Some minor comments.
await tester.enterText(find.byType(TextField).first, 'No match 2'); | ||
await tester.pump(); | ||
expect(tester.takeException(), isNull); | ||
}, variant: TargetPlatformVariant.desktop()); |
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.
Keyboard navigation is also supported on mobile (Android for sure, iOS probably).
I tried changing TargetPlatformVariant.desktop()
to TargetPlatformVariant.all()
and the test still passed.
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.
The different is requestFocusOnTap
default to false on mobile, The widget won't gain focus and won't receive key event. (I think this might be why other keyboard navigation tests used TargetPlatformVariant.desktop()
)
I can see setting requestFocusOnTap
to true
being overall the better than setting TargetPlatformVariant
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 the explanation. 🙏
For the moment, it makes sense to do the same as the other existing tests.
We can revisit this on a separate "test only" PR (feel free to ping me if you file one, I like PRs that aim at improving existing tests 🔥 ), setting requestFocusOnTap
to true
seems to be a nice option (a comment might be interesting to remind why).
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.
Oops sorry, I was reading the wrong version. I see now that you applies this change, it is ok to me. Feel free to add a comment if you think it worths it.
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, I've changed it back to TargetPlatformVariant.desktop()
for now.
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.
Oops, it appears to be some delays and I didn't see the new message in time, I will try to file a PR about overall requestFocusOnTap
in keyboard navigation in a few days.
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 taking care of this fix
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! Great to see a 1-line fix 🙂👍
I accidentally submitted the wrong test in the last commit in #155252 Re-uploading the correct one.
…7815) Manual roll Flutter from 0975e612c04a to ec2e12ba5099 (54 revisions) Manual roll requested by stuartmorgan@google.com flutter/flutter@0975e61...ec2e12b 2024-10-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from bd44b58e3204 to 247bc68c578e (7 revisions) (flutter/flutter#156144) 2024-10-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 70232fa124d0 to bd44b58e3204 (1 revision) (flutter/flutter#156124) 2024-10-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 33ac1b30ab0a to 70232fa124d0 (2 revisions) (flutter/flutter#156122) 2024-10-02 36861262+QuncCccccc@users.noreply.github.com Update `ThemeData.dialogTheme` type to accept `DialogThemeData` (flutter/flutter#155129) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 751ab9b3c5eb to 33ac1b30ab0a (4 revisions) (flutter/flutter#156118) 2024-10-02 aam@google.com Add back main() methods to benchmark benches. (flutter/flutter#156083) 2024-10-02 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#156117) 2024-10-02 victorsanniay@gmail.com Add `mouseCursor` property to `CupertinoCheckbox` (flutter/flutter#151788) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3bdc1c0a30b6 to 751ab9b3c5eb (3 revisions) (flutter/flutter#156115) 2024-10-02 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#156114) 2024-10-02 bkonyi@google.com [ Cocoon ] Wait for task results to be received by the task runner before shutting down the task process (flutter/flutter#156002) 2024-10-02 58190796+MitchellGoodwin@users.noreply.github.com Allow mixing route transitions in one app. (flutter/flutter#150031) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from f20681241753 to 3bdc1c0a30b6 (5 revisions) (flutter/flutter#156107) 2024-10-02 reidbaker@google.com Update Upgrading-Engine's-Android-API-version.md to reflect code move (flutter/flutter#156108) 2024-10-02 engine-flutter-autoroll@skia.org Roll Packages from ebcc4f0 to 7c97c88 (5 revisions) (flutter/flutter#156106) 2024-10-02 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#156105) 2024-10-02 120297255+PurplePolyhedron@users.noreply.github.com fix wrong test in "fixing `DropdownMenu` keyboard navigation" (flutter/flutter#156084) 2024-10-02 brackenavaron@gmail.com fix ReorderableList not passing in item extent builder (flutter/flutter#155994) 2024-10-02 98614782+auto-submit[bot]@users.noreply.github.com Reverts "integration_test: migrate to build.gradle.kts (#154125)" (flutter/flutter#156087) 2024-10-02 magder@google.com Add deprecation warning for "flutter create --ios-language" (flutter/flutter#155867) 2024-10-02 devoncarew@google.com update flutter create generated projects to use package:flutter_lints 5.0.0 (flutter/flutter#156011) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from d48c35d16814 to f20681241753 (1 revision) (flutter/flutter#156080) 2024-10-02 captainsikandar47@gmail.com [Docs] `CupertinoListTile` API Example (flutter/flutter#154548) 2024-10-02 barpac02@gmail.com integration_test: migrate to build.gradle.kts (flutter/flutter#154125) 2024-10-02 fluttergithubbot@gmail.com Marks Windows_mokey native_assets_android to be flaky (flutter/flutter#156064) 2024-10-02 leroux_bruno@yahoo.fr Fix DropdownMenu does not rematch initialSelection when entries have changed (flutter/flutter#155757) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9b224bd2f895 to d48c35d16814 (1 revision) (flutter/flutter#156074) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8774940b9ddc to 9b224bd2f895 (1 revision) (flutter/flutter#156065) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 21ad04948457 to 8774940b9ddc (1 revision) (flutter/flutter#156055) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 767bdc38cf51 to 21ad04948457 (1 revision) (flutter/flutter#156049) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 055969512dc5 to 767bdc38cf51 (1 revision) (flutter/flutter#156043) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from e0f049d69240 to 055969512dc5 (2 revisions) (flutter/flutter#156042) 2024-10-02 120297255+PurplePolyhedron@users.noreply.github.com fix `DropdownMenu` keyboard navigation when entries are filtered empty (flutter/flutter#155252) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from a5bc2e2708c7 to e0f049d69240 (1 revision) (flutter/flutter#156039) 2024-10-02 christopherfujino@gmail.com mark {Linux,Windows} tool_integration_tests_* non-bringup (flutter/flutter#155773) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from a7abf7a8163e to a5bc2e2708c7 (2 revisions) (flutter/flutter#156038) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from df1982dd4482 to a7abf7a8163e (1 revision) (flutter/flutter#156032) 2024-10-02 66697085+SuicaLondon@users.noreply.github.com Add `enableSplash` parameter to `CarouselView` (flutter/flutter#155214) 2024-10-02 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#156030) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from ab48d6d8c167 to df1982dd4482 (1 revision) (flutter/flutter#156029) 2024-10-01 fluttergithubbot@gmail.com Marks Mac_arm64_ios hot_mode_dev_cycle_ios__benchmark to be unflaky (flutter/flutter#147289) 2024-10-01 sokolovskyi.konstantin@gmail.com Add WidgetStateMouseCursor example and tests for it. (flutter/flutter#155552) 2024-10-01 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.5.0 to 4.6.0 (flutter/flutter#156024) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from e7b3ce717006 to ab48d6d8c167 (1 revision) (flutter/flutter#156023) ...
fixes #154532
Pre-launch Checklist
///
).If you need help, consider asking for advice on the #hackers-new channel on Discord.