Autocomplete keyboard navigation #159455

justinmc · 2024-11-26T00:26:14Z

I noticed some problems with Autocomplete keyboard navigation:

Previously, when wrapping from the top to bottom or vice-versa, scrolling wouldn't happen until after you hit the arrow key a few extra times. This is due to the way we were doing scrolling and the fact that those items were not yet built by ListView.builder. I fixed this.

Video of scrolling bug

Screencast.from.2024-11-25.16-21-06.webm

This wrapping feature itself seemed to not be part of the <select> tag on web (try it on MDN), which Autocomplete is kind of based on, so I removed it. It now stops at the top/bottom.
We were missing a couple of keyboard shortcuts (Advanced Autocomplete keyboard controls #85233)

Fixes #85233

Trying out <select> on web, it doesn't wrap, so let's not wrap either.

But maybe I want to test this with the highlight index instead of submitting each time...

victorsanni · 2024-11-26T21:35:02Z

Should this land before or after #143249?

justinmc · 2024-11-26T22:51:29Z

Ah I was hoping this would be totally separate and not conflict, but I do see merge conflicts if I try to merge one into the other. Is #143249 close to merging? I don't want to delay that any further, so this one can probably wait.

victorsanni

I don't fully understand actions/intents in Flutter, but the approach and tests lgtm.

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

justinmc · 2025-01-15T21:50:37Z

Updated to the monorepo and autoformatter, but waiting for #143249 to be relanded.

victorsanni

Not very knowledgeable about actions/intents, but the autocomplete parts lgtm. How much of a breaking change is the options list no longer wrapping at the ends?

victorsanni · 2025-01-21T23:27:45Z

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

    debugLabel: '_RawAutocompleteState',
  );

+  static const int _pageSize = 4;


There should be a comment here explaining what this variable means.

justinmc · 2025-01-21T23:49:37Z

@Renzo-Olivares Could you do the secondary review here just to make sure the action/intent stuff looks good?

justinmc · 2025-01-21T23:52:14Z

How much of a breaking change is the options list no longer wrapping at the ends?

It seems to not break any Google or customer tests. I adjusted two places in our tests that broke when I made the change. I think I wrote that wrapping behavior just off the top of my head without trying <select> or anything, so I'm hoping it's an easy change.

Renzo-Olivares

Actions/intents LGTM

I noticed some problems with Autocomplete keyboard navigation: * Previously, when wrapping from the top to bottom or vice-versa, scrolling wouldn't happen until after you hit the arrow key a few extra times. This is due to the way we were doing scrolling and the fact that those items were not yet built by ListView.builder. I fixed this. <details> <summary>Video of scrolling bug</summary> [Screencast from 2024-11-25 16-21-06.webm](https://github.com/user-attachments/assets/ce01065a-1a03-4f37-b0f6-2d6273852ac4) </details> * This wrapping feature itself seemed to not be part of the `<select>` tag on web ([try it on MDN](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/select)), which Autocomplete is kind of based on, so I removed it. It now stops at the top/bottom. * We were missing a couple of keyboard shortcuts (flutter#85233) Fixes flutter#85233

Manual roll requested by [email protected] flutter/flutter@c1561a4...c1ffaa9 2025-01-24 [email protected] Fix link to hotfix documentation best practices (flutter/flutter#162116) 2025-01-24 [email protected] Add integration test for cutout rotation evaluation (flutter/flutter#160354) 2025-01-24 [email protected] Reland "[Impeller] Migrate unit tests off of Skia geometry classes (#161855)" (flutter/flutter#162146) 2025-01-24 [email protected] Fix TextField intrinsic width when hint is not visible (flutter/flutter#161235) 2025-01-24 [email protected] When parsing flavors, handle Xcode build configurations that are not lowercase (flutter/flutter#161455) 2025-01-24 [email protected] [Impeller] Fix source offset in PathBuilder::AddPath (flutter/flutter#162052) 2025-01-24 [email protected] Add to Setup Path Example to Engine README (flutter/flutter#162115) 2025-01-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Unskip test. (#162106)" (flutter/flutter#162122) 2025-01-23 [email protected] feat: Add `hint` (Widget) property to InputDecoration (flutter/flutter#161424) 2025-01-23 [email protected] Fix skwasm target in wasm_debug_unopt build. (flutter/flutter#162100) 2025-01-23 [email protected] Marks Linux_android_emu android views to be unflaky (flutter/flutter#160493) 2025-01-23 [email protected] Unskip test. (flutter/flutter#162106) 2025-01-23 [email protected] Add ability to maintain bottom view padding in `NavigationBar` safe area (flutter/flutter#162076) 2025-01-23 [email protected] Roll pub packages (flutter/flutter#162095) 2025-01-23 [email protected] Delete an unused (manual) workflow, added missing copyright headers. (flutter/flutter#162050) 2025-01-23 [email protected] Android templates: update default Kotlin from 1.8.22 to 2.1.0, update default Gradle from 8.9 to 8.12 (flutter/flutter#160974) 2025-01-23 [email protected] flutter_tools: flutter_tester is a host artifact (flutter/flutter#162047) 2025-01-23 [email protected] [Impeller] Make glIsTexture mockable for use by the ReactorGLES.NameUntrackedHandle test (flutter/flutter#162082) 2025-01-23 [email protected] Remove "Mac Designed for iPad" as a discoverable `flutter run` device (flutter/flutter#161459) 2025-01-23 [email protected] Show error on macOS if missing Local Network permissions (flutter/flutter#161846) 2025-01-23 [email protected] Autocomplete keyboard navigation (flutter/flutter#159455) 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] 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

) Manual roll requested by [email protected] flutter/flutter@c1561a4...c1ffaa9 2025-01-24 [email protected] Fix link to hotfix documentation best practices (flutter/flutter#162116) 2025-01-24 [email protected] Add integration test for cutout rotation evaluation (flutter/flutter#160354) 2025-01-24 [email protected] Reland "[Impeller] Migrate unit tests off of Skia geometry classes (#161855)" (flutter/flutter#162146) 2025-01-24 [email protected] Fix TextField intrinsic width when hint is not visible (flutter/flutter#161235) 2025-01-24 [email protected] When parsing flavors, handle Xcode build configurations that are not lowercase (flutter/flutter#161455) 2025-01-24 [email protected] [Impeller] Fix source offset in PathBuilder::AddPath (flutter/flutter#162052) 2025-01-24 [email protected] Add to Setup Path Example to Engine README (flutter/flutter#162115) 2025-01-23 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Unskip test. (#162106)" (flutter/flutter#162122) 2025-01-23 [email protected] feat: Add `hint` (Widget) property to InputDecoration (flutter/flutter#161424) 2025-01-23 [email protected] Fix skwasm target in wasm_debug_unopt build. (flutter/flutter#162100) 2025-01-23 [email protected] Marks Linux_android_emu android views to be unflaky (flutter/flutter#160493) 2025-01-23 [email protected] Unskip test. (flutter/flutter#162106) 2025-01-23 [email protected] Add ability to maintain bottom view padding in `NavigationBar` safe area (flutter/flutter#162076) 2025-01-23 [email protected] Roll pub packages (flutter/flutter#162095) 2025-01-23 [email protected] Delete an unused (manual) workflow, added missing copyright headers. (flutter/flutter#162050) 2025-01-23 [email protected] Android templates: update default Kotlin from 1.8.22 to 2.1.0, update default Gradle from 8.9 to 8.12 (flutter/flutter#160974) 2025-01-23 [email protected] flutter_tools: flutter_tester is a host artifact (flutter/flutter#162047) 2025-01-23 [email protected] [Impeller] Make glIsTexture mockable for use by the ReactorGLES.NameUntrackedHandle test (flutter/flutter#162082) 2025-01-23 [email protected] Remove "Mac Designed for iPad" as a discoverable `flutter run` device (flutter/flutter#161459) 2025-01-23 [email protected] Show error on macOS if missing Local Network permissions (flutter/flutter#161846) 2025-01-23 [email protected] Autocomplete keyboard navigation (flutter/flutter#159455) 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] 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

justinmc added 9 commits November 25, 2024 13:53

Not AutoComplete!

3933524

Scrolls to top/bottom as well

bd4e9ed

ctrl + up/down

5abed68

PageDown, and don't allow wrapping.

6c2b2aa

Trying out <select> on web, it doesn't wrap, so let's not wrap either.

PageUp

25ce553

Meta key apple shortcuts

dddc5c6

Fix tests and write tests for new keys

87b0e0d

But maybe I want to test this with the highlight index instead of submitting each time...

Used highlight index for better tests

08c2439

Test bugfix for jumping to unbuilt options

509edda

justinmc requested a review from victorsanni November 26, 2024 00:26

justinmc self-assigned this Nov 26, 2024

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

analyzer

f9b4195

victorsanni reviewed Nov 26, 2024

View reviewed changes

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

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

justinmc added 4 commits December 2, 2024 10:58

Forgot to delete this comment

f056466

Merge branch 'master' into autocomplete-keyboard

0475d41

Merge branch 'master' into autocomplete-keyboard

62a2072

autoformat

a531110

Merge branch 'master' into autocomplete-keyboard

adbffeb

justinmc requested a review from victorsanni January 21, 2025 23:23

victorsanni approved these changes Jan 21, 2025

View reviewed changes

Review: comment explaining _pageSize

8731352

justinmc requested a review from Renzo-Olivares January 21, 2025 23:49

Renzo-Olivares approved these changes Jan 23, 2025

View reviewed changes

justinmc added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 23, 2025

auto-submit bot added this pull request to the merge queue Jan 23, 2025

Merged via the queue into flutter:master with commit 3baa356 Jan 23, 2025
104 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 23, 2025

justinmc deleted the autocomplete-keyboard branch January 23, 2025 19:45

engine-flutter-autoroll mentioned this pull request Jan 24, 2025

Roll Flutter from c1561a41c530 to bf7daf22a51e (19 revisions) flutter/packages#8495

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 24, 2025

3baa356 Autocomplete keyboard navigation (flutter/flutter#159455)

f3933f8

engine-flutter-autoroll mentioned this pull request Jan 24, 2025

Roll Flutter from c1561a41c530 to 02e11f95a18a (20 revisions) flutter/packages#8496

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 24, 2025

3baa356 Autocomplete keyboard navigation (flutter/flutter#159455)

98d88fd

engine-flutter-autoroll mentioned this pull request Jan 24, 2025

Roll Flutter from c1561a41c530 to c1ffaa9d9deb (21 revisions) flutter/packages#8497

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 24, 2025

3baa356 Autocomplete keyboard navigation (flutter/flutter#159455)

bf32cf0

engine-flutter-autoroll mentioned this pull request Jan 24, 2025

Manual roll Flutter from c1561a41c530 to c1ffaa9d9deb (21 revisions) flutter/packages#8498

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 24, 2025

3baa356 Autocomplete keyboard navigation (flutter/flutter#159455)

dad0cc0

engine-flutter-autoroll mentioned this pull request Jan 24, 2025

Roll Flutter from c1561a41c530 to c1ffaa9d9deb (21 revisions) flutter/packages#8500

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 24, 2025

3baa356 Autocomplete keyboard navigation (flutter/flutter#159455)

f2e8511

Piinks mentioned this pull request Jan 24, 2025

Flutter Notable Commits - 2025 #161460

Open

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

3baa356 Autocomplete keyboard navigation (flutter/flutter#159455)

a4d93d4

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

3baa356 Autocomplete keyboard navigation (flutter/flutter#159455)

94184d8

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

3baa356 Autocomplete keyboard navigation (flutter/flutter#159455)

164fdea

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Autocomplete keyboard navigation #159455

Autocomplete keyboard navigation #159455

Uh oh!

justinmc commented Nov 26, 2024

Uh oh!

victorsanni commented Nov 26, 2024

Uh oh!

justinmc commented Nov 26, 2024

Uh oh!

victorsanni left a comment

Uh oh!

Uh oh!

Uh oh!

justinmc commented Jan 15, 2025

Uh oh!

victorsanni left a comment

Uh oh!

victorsanni Jan 21, 2025

Uh oh!

justinmc commented Jan 21, 2025

Uh oh!

justinmc commented Jan 21, 2025

Uh oh!

Renzo-Olivares left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Autocomplete keyboard navigation #159455

Autocomplete keyboard navigation #159455

Uh oh!

Conversation

justinmc commented Nov 26, 2024

Uh oh!

victorsanni commented Nov 26, 2024

Uh oh!

justinmc commented Nov 26, 2024

Uh oh!

victorsanni left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

justinmc commented Jan 15, 2025

Uh oh!

victorsanni left a comment

Choose a reason for hiding this comment

Uh oh!

victorsanni Jan 21, 2025

Choose a reason for hiding this comment

Uh oh!

justinmc commented Jan 21, 2025

Uh oh!

justinmc commented Jan 21, 2025

Uh oh!

Renzo-Olivares left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants