Fix `DropdownMenu` keyboard navigation #147294

PurplePolyhedron · 2024-04-24T11:39:49Z

Resubmitted #147285 because I accidentally added reviewer as contributor in a commit by using Github "add suggestion to batch". Which causing CLA check to fail and I cannot revert it.

I use exact match of label instead of contains to minimise possible change in actual user search.
I added the new test after the original keyboard navigation test to avoid merge conflict with another incoming PR.
Let me know if I need to make any changes.

Sorry for the inconvenience, I am new to the process.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

PurplePolyhedron · 2024-04-24T12:01:42Z

cc @QuncCccccc who has a great knowledge of this widget.

as suggested in the original PR.

nate-thegrate

Thanks for this contribution!

packages/flutter/lib/src/material/dropdown_menu.dart

QuncCccccc

Thanks for the contribution and this looks great! Just left one question below:)

packages/flutter/lib/src/material/dropdown_menu.dart

test side effect of change search to contain

QuncCccccc

LGTM:) Thanks for the fix!

packages/flutter/test/material/dropdown_menu_test.dart

QuncCccccc

LGTM! Thanks for the contribution!

packages/flutter/test/material/dropdown_menu_test.dart

flutter/flutter@2bfb1b0...2aa05c1 2024-05-11 [email protected] Roll Flutter Engine from fad88cb16d03 to 558a81dd8b08 (3 revisions) (flutter/flutter#148163) 2024-05-11 [email protected] Roll Flutter Engine from ba8e0d3e2f23 to fad88cb16d03 (9 revisions) (flutter/flutter#148156) 2024-05-11 [email protected] Add test for scaffold.1.dart (flutter/flutter#147966) 2024-05-10 [email protected] Fix `MaterialStateBorderSide` lerp in the `Checkbox` and chips (flutter/flutter#148124) 2024-05-10 [email protected] Docs on TextField disposed by a scrollable (flutter/flutter#148149) 2024-05-10 [email protected] Roll Flutter Engine from d4f705ccb695 to ba8e0d3e2f23 (8 revisions) (flutter/flutter#148147) 2024-05-10 [email protected] Roll pub packages (flutter/flutter#148148) 2024-05-10 [email protected] Add `clipBehavior` to `DialogTheme` (flutter/flutter#147635) 2024-05-10 [email protected] bump cupertino_icons to 1.08 (flutter/flutter#146806) 2024-05-10 [email protected] Add test for animated_size.0.dart API example. (flutter/flutter#147828) 2024-05-10 [email protected] Fix `DropdownMenu` keyboard navigation (flutter/flutter#147294) 2024-05-10 [email protected] Add test for draggable.0.dart API example. (flutter/flutter#147941) 2024-05-10 [email protected] Update TESTOWNERS (flutter/flutter#148108) 2024-05-10 [email protected] Add tests for stream_builder.0.dart API example. (flutter/flutter#147832) 2024-05-10 [email protected] Roll Flutter Engine from 1ccd0c308b3a to d4f705ccb695 (2 revisions) (flutter/flutter#148142) 2024-05-10 [email protected] Roll Packages from 8de142d to 6c4482a (8 revisions) (flutter/flutter#148079) 2024-05-10 [email protected] Roll Flutter Engine from c0917b14fc36 to 1ccd0c308b3a (10 revisions) (flutter/flutter#148137) 2024-05-10 [email protected] `if` chains â�� `switch` expressions (flutter/flutter#147793) 2024-05-10 49699333+dependabot[bot]@users.noreply.github.com Bump ossf/scorecard-action from 2.3.1 to 2.3.3 (flutter/flutter#148091) 2024-05-10 [email protected] Reland "Implement computeDryBaseline for `RenderWrap` (#146260)" (flutter/flutter#148086) 2024-05-10 [email protected] Update dependabot reviewers (flutter/flutter#148101) 2024-05-10 [email protected] Roll Flutter Engine from 6e722ae213bd to c0917b14fc36 (1 revision) (flutter/flutter#148084) 2024-05-09 [email protected] Don't pin package:macros (flutter/flutter#148087) 2024-05-09 [email protected] Remove hidden dependencies on the default LocalPlatform (flutter/flutter#147342) 2024-05-09 [email protected] Getting rid of containers (flutter/flutter#147432) 2024-05-09 [email protected] Roll Flutter Engine from c0fd3386d018 to 6e722ae213bd (2 revisions) (flutter/flutter#148070) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

fixes #152375 PR #147294 fix keyboard navigation issue #147253 by prioritising current selection in default search. This revert PR #147294 and fix the issue by disabling `DropdownMenu` search during keyboard navigation instead. also fixes #151878 as it is a regression caused by PR #147294. Reason for reverting #147294 : it doesn't fix the keyboard navigation issue if a custom searchCallback(Issue #152375) is provided, and "prioritising current selection" is ambiguous for a custom searchCallback, so it would be tricky to have searchCallback behave the same way as the default search. Additionally the original search function was agnostic about the state of `DropdownMenu` other than the entry value type, might be good to keep it that way unless needed.

fixes flutter#152375 PR flutter#147294 fix keyboard navigation issue flutter#147253 by prioritising current selection in default search. This revert PR flutter#147294 and fix the issue by disabling `DropdownMenu` search during keyboard navigation instead. also fixes flutter#151878 as it is a regression caused by PR flutter#147294. Reason for reverting flutter#147294 : it doesn't fix the keyboard navigation issue if a custom searchCallback(Issue flutter#152375) is provided, and "prioritising current selection" is ambiguous for a custom searchCallback, so it would be tricky to have searchCallback behave the same way as the default search. Additionally the original search function was agnostic about the state of `DropdownMenu` other than the entry value type, might be good to keep it that way unless needed.

PurplePolyhedron added 2 commits April 24, 2024 21:15

fix dropdown menu keyboard navigation test

f4e0ea9

Add files via upload

8b82fe7

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Apr 24, 2024

PurplePolyhedron mentioned this pull request Apr 24, 2024

Fix dropdown menu keyboard navigation #147285

Closed

9 tasks

bleroux requested a review from QuncCccccc April 24, 2024 12:26

nate-thegrate approved these changes May 2, 2024

View reviewed changes

packages/flutter/lib/src/material/dropdown_menu.dart Outdated Show resolved Hide resolved

QuncCccccc reviewed May 8, 2024

View reviewed changes

packages/flutter/lib/src/material/dropdown_menu.dart Outdated Show resolved Hide resolved

PurplePolyhedron added 4 commits May 9, 2024 14:43

Merge branch 'flutter:master' into dropdown_menu_keyboard_navigation2

4d71e0e

change search to contain

1122eec

change search to contain test

fbe6b3f

test side effect of change search to contain

format typo

dc87c7f

QuncCccccc approved these changes May 9, 2024

View reviewed changes

QuncCccccc approved these changes May 10, 2024

View reviewed changes

packages/flutter/test/material/dropdown_menu_test.dart Show resolved Hide resolved

QuncCccccc added the autosubmit Merge PR when tree becomes green via auto submit App label May 10, 2024

auto-submit bot merged commit fdfc408 into flutter:master May 10, 2024

engine-flutter-autoroll mentioned this pull request May 11, 2024

Roll Flutter from 2bfb1b0e2f61 to 839ba6b3e395 (25 revisions) flutter/packages#6712

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 11, 2024

fdfc408 Fix DropdownMenu keyboard navigation (flutter/flutter#147294)

e1c3451

engine-flutter-autoroll mentioned this pull request May 11, 2024

Roll Flutter from 2bfb1b0e2f61 to 2aa05c16509b (26 revisions) flutter/packages#6713

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 11, 2024

fdfc408 Fix DropdownMenu keyboard navigation (flutter/flutter#147294)

11f1319

Piinks mentioned this pull request Jul 18, 2024

DropdownMenu throws RangeError when both filter and search are enabled in latest beta and master. #151878

Closed

bleroux mentioned this pull request Jul 22, 2024

fix: Dropdown menu trying to access highlight element which doesn't exist when search and filters both are enabled #151969

Merged

9 tasks

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 6, 2024

fdfc408 Fix DropdownMenu keyboard navigation (flutter/flutter#147294)

6626324

PurplePolyhedron mentioned this pull request Sep 12, 2024

[CP] fix DropdownMenu range error when filter are enabled #155078

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix `DropdownMenu` keyboard navigation #147294

Fix `DropdownMenu` keyboard navigation #147294

Uh oh!

PurplePolyhedron commented Apr 24, 2024 •

edited by nate-thegrate

Loading

Uh oh!

PurplePolyhedron commented Apr 24, 2024

Uh oh!

nate-thegrate left a comment

Uh oh!

Uh oh!

QuncCccccc left a comment

Uh oh!

Uh oh!

QuncCccccc left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

QuncCccccc left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix DropdownMenu keyboard navigation #147294

Fix DropdownMenu keyboard navigation #147294

Uh oh!

Conversation

PurplePolyhedron commented Apr 24, 2024 • edited by nate-thegrate Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

PurplePolyhedron commented Apr 24, 2024

Uh oh!

nate-thegrate left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

QuncCccccc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

QuncCccccc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

QuncCccccc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix `DropdownMenu` keyboard navigation #147294

Fix `DropdownMenu` keyboard navigation #147294

PurplePolyhedron commented Apr 24, 2024 •

edited by nate-thegrate

Loading