Skip to content
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

fix(list-key-manager): remove handling for home and end keys #4544

Merged

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented May 14, 2017

  • Removes the handling for the home and end keys from the ListKeyManager since their usage isn't as generic as the arrow keys.
  • Adds the home and end key handling to the select specifically.
  • Reworks the Autocomplete, ListKeyManager and ChipList tests to use the createKeyboardEvent, instead of making their own fake keyboard events.
  • Adds a target parameter to the createKeyboardEvent function, allowing us to define the event target.

Fixes #3496.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label May 14, 2017
@jelbourn
Copy link
Member

I did a quick survey of Google Search, GMail, Drive, and this accessible autocomplete for US government web sites. For all of them, the home/end keys always apply to the text input no mater what state you're in. I think we should just follow this pattern, since it will be simpler and follow conventions.

@crisbeto crisbeto force-pushed the 3496/list-key-manager-interactive branch from d56e5d2 to 0083b9c Compare May 16, 2017 18:23
@crisbeto crisbeto changed the title fix(list-key-manager): don't handle home and end keys unless the user is interacting with the element fix(list-key-manager): remove handling for home and end keys May 16, 2017
@crisbeto
Copy link
Member Author

Done @jelbourn. I removed it from the ListKeyManager and re-added it only to MdSelect since the native select does handle home and end.

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, one minor comment

let TAB_EVENT: KeyboardEvent;
let HOME_EVENT: KeyboardEvent;
let END_EVENT: KeyboardEvent;
let EVENTS: {
Copy link
Member

Choose a reason for hiding this comment

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

How about just fakeKeyEvents? It's not constant since you write the value in beforeEach.

@crisbeto crisbeto added the Accessibility This issue is related to accessibility (a11y) label May 18, 2017
@crisbeto crisbeto force-pushed the 3496/list-key-manager-interactive branch from 0083b9c to 86229f4 Compare May 18, 2017 03:54
@crisbeto crisbeto added action: merge The PR is ready for merge by the caretaker and removed pr: needs rebase labels May 18, 2017
@kara kara removed their assignment May 19, 2017
* Removes the handling for the home and end keys from the ListKeyManager since their usage isn't as generic as the arrow keys.
* Adds the home and end key handling to the select specifically.
* Reworks the Autocomplete, ListKeyManager and ChipList tests to use the `createKeyboardEvent`, instead of making their own fake keyboard events.
* Adds a `target` parameter to the `createKeyboardEvent` function, allowing us to define the event target.

Fixes angular#3496.
@crisbeto crisbeto force-pushed the 3496/list-key-manager-interactive branch from 86229f4 to 3cf0f7d Compare May 20, 2017 20:37
@tinayuangao tinayuangao merged commit 2d16345 into angular:master May 22, 2017
@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 Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Accessibility This issue is related to accessibility (a11y) action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

autocomplete - HOME and END keys not working
5 participants