Skip to content

feat(a11y): Add optional home/end key support to ListKeyManager #19834

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

Merged
merged 1 commit into from
Jul 11, 2020

Conversation

vanessanschmitt
Copy link
Collaborator

@vanessanschmitt vanessanschmitt commented Jul 1, 2020

For some components like menus and listboxes, WAI a11y recommends
that pressing home or end should focus the first or last element
respectively. Add support for home/end to ListKeyManager.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 1, 2020
For some components like menus and listboxes, WAI a11y recommends
that pressing home or end should focus the first or last element
respectively. Add support for home/end to ListKeyManager.
@jelbourn
Copy link
Member

jelbourn commented Jul 2, 2020

Thoughts on making this the default behavior? I'm actually surprised that we didn't do this already.

Copy link
Member

@crisbeto crisbeto left a comment

Choose a reason for hiding this comment

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

We used to have this inside the ListKeyManager, but we slowly transitioned to doing it on a case-by-case basis, because there are some places where home/end already do something. E.g. in an input they were conflicting with the native behavior that moves the caret to the start/end.

@vanessanschmitt
Copy link
Collaborator Author

Thanks for the context, @crisbeto! That makes sense why we wouldn't want this to be the default behavior. I'm migrating a bunch of my team's components to use the cdk KeyManager classes, and one of my main justifications is "Look how many lines of custom code I removed!" haha. A lot of them have duplicate Home/End key logic, so I'd love to be able to remove that too by making this an option in ListKeyManager...

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added lgtm action: merge The PR is ready for merge by the caretaker target: minor This PR is targeted for the next minor release labels Jul 6, 2020
@jelbourn
Copy link
Member

jelbourn commented Jul 8, 2020

@googlebot Ping

@mmalerba mmalerba merged commit 544e335 into angular:master Jul 11, 2020
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jul 13, 2020
Based on top of angular#19834, changes the places where we were handling home/end to go through the key manager.

Also allows `withHomeAndEnd` to be turned off, similarly to what we're doing with the other methods.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jul 13, 2020
Based on top of angular#19834, changes the places where we were handling home/end to go through the key manager.

Also allows `withHomeAndEnd` to be turned off, similarly to what we're doing with the other methods.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Jul 22, 2020
Based on top of angular#19834, changes the places where we were handling home/end to go through the key manager.

Also allows `withHomeAndEnd` to be turned off, similarly to what we're doing with the other methods.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Aug 2, 2020
Based on top of angular#19834, changes the places where we were handling home/end to go through the key manager.

Also allows `withHomeAndEnd` to be turned off, similarly to what we're doing with the other methods.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Aug 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants