Fix dropdown keys to open menu #32750
Conversation
sijusamson
force-pushed
the
dropdown_keys_no_longer_open_menu
branch
from
0ebc541
to
3bc08de
Compare
XhmikosR
changed the title
|
Can you add a test please? This wasn't caught. |
sijusamson
force-pushed
the
dropdown_keys_no_longer_open_menu
branch
from
3bc08de
to
b2c710b
Compare
Done ! |
|
Works good, thanks! @patrickhlauke anything a11y-wise we might be missing? |
|
It's more a nice to have than a necessary (from a11y point of view) feature, but conversely it's not introducing any issues either. Long way of saying LGTM. |
| if (!isActive && (event.key === ARROW_UP_KEY || event.key === ARROW_DOWN_KEY)) { | ||
| const button = this.matches(SELECTOR_DATA_TOGGLE) ? this : SelectorEngine.prev(this, SELECTOR_DATA_TOGGLE)[0] | ||
| button.click() | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
Now the method dataApiKeydownHandler has a linting warning:
Static method 'dataApiKeydownHandler' has a complexity of 23. Maximum allowed is 20.
There was a problem hiding this comment.
@rohit2sharma95 Thank you for your suggestion, let me try to figure out the best possible solution.
There was a problem hiding this comment.
As per my findings, this linting warning can be removed if we split the function dataApiKeydownHandler. We can create a new static method, and pass on some functionality there.
But should we do that ?
@rohit2sharma95 @patrickhlauke @XhmikosR
There was a problem hiding this comment.
I'd say ignore it for now and tackle it later in another PR.
sijusamson
force-pushed
the
dropdown_keys_no_longer_open_menu
branch
from
e0b1cc7
to
767c4be
Compare
Up/Down Key not working for dropdown
sijusamson
force-pushed
the
dropdown_keys_no_longer_open_menu
branch
from
767c4be
to
7927a8f
Compare
|
BTW does it make sense to open the dropdown but not select the first/last element depending on the arrow key? I feel like it's an extra key. But that's for later too. :) |
Fixes #32714.