Skip to content

Add Enter and Tab back in to ios and macos key maps#81865

Merged
gspencergoog merged 3 commits intoflutter:masterfrom
gspencergoog:fix_tab_keycodes
May 11, 2021
Merged

Add Enter and Tab back in to ios and macos key maps#81865
gspencergoog merged 3 commits intoflutter:masterfrom
gspencergoog:fix_tab_keycodes

Conversation

@gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented May 4, 2021 •
edited
Loading

Description

I noticed that tab traversal stopped working on iOS when #73440 was landed.

It seems to be that the control characters were excluded from the list of logical keys, and so they ended up being generated values.

This PR just adds "Enter" and "Tab" to the list for both macOS and iOS.

Tests

  • Added a test for control characters to check their ID values to make sure they're using their ASCII values as the base for their ID and not something else.

@flutter-dashboard flutter-dashboard bot added framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels May 4, 2021
@google-cla google-cla bot added the cla: yes label May 4, 2021
@gspencergoog gspencergoog requested a review from dkwingsmt May 4, 2021 22:13
@gspencergoog gspencergoog marked this pull request as ready for review May 4, 2021 22:51
@gspencergoog
Copy link
Contributor Author

gspencergoog commented May 4, 2021

@dkwingsmt I noticed when I re-ran the script that I get some differences in the generated engine code too, including some that are somewhat unexpected: I assume I should be updating those too, right?

@dkwingsmt
Copy link
Contributor

dkwingsmt commented May 5, 2021

@gspencergoog I have some codegen changes collected in #81678. Let me submit that.

@fluttergithubbot
Copy link
Contributor

fluttergithubbot commented May 7, 2021

This pull request is not suitable for automatic merging in its current state.

  • Please get at least one approved review before re-applying this label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

dkwingsmt
dkwingsmt approved these changes May 8, 2021
View reviewed changes
Copy link
Contributor

@dkwingsmt dkwingsmt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I was reported this bug on macOS just now.

dkwingsmt
dkwingsmt reviewed May 8, 2021
View reviewed changes
packages/flutter/test/services/keyboard_key_test.dart Outdated
Copy link
Contributor

@dkwingsmt dkwingsmt May 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why you think this test is necessary?

Copy link
Contributor Author

@gspencergoog gspencergoog May 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because each PR needs to test what it is fixing.

This is testing for the failure of the generation code to generate the correct IDs for important values in keyboard navigation, which is what this PR is fixing. If one of those important values is missing or incorrect, the test will fail.

@gspencergoog gspencergoog merged commit af3337b into flutter:master May 11, 2021
@gspencergoog gspencergoog deleted the fix_tab_keycodes branch May 11, 2021 17:23
@YeungKC YeungKC mentioned this pull request Jun 2, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dkwingsmt dkwingsmt dkwingsmt approved these changes

Assignees

No one assigned

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@gspencergoog @dkwingsmt @fluttergithubbot