Skip to content

Re-Land: Add focus nodes, hover, and shortcuts to switches, checkboxes, and radio buttons.#43657

Merged
gspencergoog merged 2 commits intoflutter:masterfrom
gspencergoog:switch_focus
Oct 30, 2019
Merged

Re-Land: Add focus nodes, hover, and shortcuts to switches, checkboxes, and radio buttons.#43657
gspencergoog merged 2 commits intoflutter:masterfrom
gspencergoog:switch_focus

Conversation

@gspencergoog
Copy link
Contributor

@gspencergoog gspencergoog commented Oct 28, 2019

This re-lands the change that adds focus nodes, hover, and shortcuts to switches, checkboxes, and radio buttons. (#43213), with fixes for the web tests that weren't enabled in the master that it was synced to when I first landed it.

@fluttergithubbot fluttergithubbot added a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. c: contributor-productivity Team-specific productivity, code health, technical debt. labels Oct 28, 2019
@gspencergoog gspencergoog force-pushed the switch_focus branch 3 times, most recently from cf961b3 to f1acd99 Compare October 28, 2019 22:22
Copy link
Contributor

@darrenaustin darrenaustin left a comment

Choose a reason for hiding this comment

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

Looking mostly at the changes from the last land attempt, it LGTM!


// On the web, enter doesn't select things, *except* in a <select>
// element, which is what a dropdown emulates.
static final Map<LogicalKeySet, Intent> _webShortcuts =<LogicalKeySet, Intent>{
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably not critical, but this does mean that non-web versions of the app will have this taking up memory, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I guess so: it's not a lot though, since it's static. I'll make it predicated on kIsWeb too and it'll take up less space then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, since it's a private static, and all uses are within if (kIsWeb){} blocks that will be elided, I'll bet it disappears entirely on non-web platforms.

@gspencergoog gspencergoog merged commit a1c5e33 into flutter:master Oct 30, 2019
@gspencergoog gspencergoog deleted the switch_focus branch November 13, 2019 23:22
Inconnu08 pushed a commit to Inconnu08/flutter that referenced this pull request Nov 26, 2019
…s, and radio buttons. (flutter#43657)

This re-lands the change that adds focus nodes, hover, and shortcuts to switches, checkboxes, and radio buttons. (flutter#43213), with fixes for the web tests that weren't enabled in the master that it was synced to when I first landed it.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) c: contributor-productivity Team-specific productivity, code health, technical debt. f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants