Skip to content

Conversation

@crisbeto
Copy link
Member

We support typing into a select to skip to an item, as well as pressing space to open the select, however if the user is typing something that has a space in it, the select will open and interrupt the user. These changes add some logic so that we don't trigger the panel while the user is typing.

Fixes #17774.

@crisbeto crisbeto added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release labels Nov 22, 2019
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 22, 2019
We support typing into a select to skip to an item, as well as pressing space to open the select, however if the user is typing something that has a space in it, the select will open and interrupt the user. These changes add some logic so that we don't trigger the panel while the user is typing.

Fixes angular#17774.
@crisbeto crisbeto force-pushed the select-shortcut-while-typing branch from 5821492 to 4988d09 Compare November 25, 2019 20:33
@crisbeto
Copy link
Member Author

I've reworked it based on the feedback @jelbourn.

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 pr: lgtm action: merge The PR is ready for merge by the caretaker labels Nov 25, 2019
it('should expose whether the user is currently typing', fakeAsync(() => {
expect(keyManager.isTyping()).toBe(false);

keyManager.onKeydown(createKeyboardEvent('keydown', 79, 'o')); // types "o"
Copy link
Member

Choose a reason for hiding this comment

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

out of curiosity: why not using cdk/keycodes for the O keycode? (for consistency)

Copy link
Member

@devversion devversion Nov 25, 2019

Choose a reason for hiding this comment

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

Nevermind.. I see other tests in this file hardcode the codes too.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think most of the tests in this file were written before we added as many constants to the CDK.

@jelbourn jelbourn merged commit 2f17450 into angular:master Nov 26, 2019
crisbeto added a commit to crisbeto/material2 that referenced this pull request Nov 27, 2019
…tion list

Along the same lines as angular#17785. Doesn't handle space key presses while the user is using the typeahead so that we don't interrupt their typing.
crisbeto added a commit to crisbeto/material2 that referenced this pull request Nov 27, 2019
…tion list

Along the same lines as angular#17785. Doesn't handle space key presses while the user is using the typeahead so that we don't interrupt their typing.
jelbourn pushed a commit that referenced this pull request Dec 3, 2019
…tion list (#17826)

Along the same lines as #17785. Doesn't handle space key presses while the user is using the typeahead so that we don't interrupt their typing.
@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 Dec 27, 2019
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 P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mat-select poorly handles "space" when performing typeahead

4 participants