fix(menu): keyboard controls not working if all items are disabled #16572
Conversation
crisbeto
added
P3
googlebot
added
the
cla: yes
Currently we depend on `focusFirstItem` to bring focus into the menu, however if all the items are disabled, focus will stay on the trigger and the user won't get feedback that something has happened. Furthermore, the keyboard shortcuts that are implemented in the menu won't work either, because the keyboard event listener is on the menu. These changes work around the issue by setting focus on the `role="menu"` if none of the items were focusable. Fixes angular#16565.
crisbeto
force-pushed
the
16565.disabled-menu-item-keyboard
branch
from
f4da270 to
f0d6f06
Compare
| // the items and walking up the DOM. | ||
| while (element) { | ||
| if (element.getAttribute('role') === 'menu') { | ||
| element.focus(); |
There was a problem hiding this comment.
Note: it felt a little weird to have this logic here since the method is called focusFirstItem, however the alternative is to do it inside the menu trigger which would require another method to be added to the MatMenuPanel interface which will increase the API surface. Furthermore, doing it through the trigger may no be very reliable, because we'd need to check if the document.activeElement is inside the menu and some browsers move focus asynchronously.
| @@ -762,6 +762,44 @@ describe('MatMenu', () => { | |||
| flush(); | |||
| })); | |||
|
|
|||
| it('should respect the DOM order, rather than insertion order, when moving focus using ' + | |||
There was a problem hiding this comment.
I ported this test over, because it got in through a very old PR that got in recently which was opened before the MDC menu existed.
jelbourn
added
pr: lgtm
action: merge
…ngular#16572) Currently we depend on `focusFirstItem` to bring focus into the menu, however if all the items are disabled, focus will stay on the trigger and the user won't get feedback that something has happened. Furthermore, the keyboard shortcuts that are implemented in the menu won't work either, because the keyboard event listener is on the menu. These changes work around the issue by setting focus on the `role="menu"` if none of the items were focusable. Fixes angular#16565.
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Currently we depend on
focusFirstItemto bring focus into the menu, however if all the items are disabled, focus will stay on the trigger and the user won't get feedback that something has happened. Furthermore, the keyboard shortcuts that are implemented in the menu won't work either, because the keyboard event listener is on the menu.These changes work around the issue by setting focus on the
role="menu"if none of the items were focusable.Fixes #16565.