material-components / material-components/material-web

Typeaheadcontroller typeahead wrongly activated on menu navigation

Open
#5,808 0 comments 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
SCSS
Stars
11.3k
Forks
1.1k
Avg merge
20h 28m
Merged PRs (30d)
13

Description

### What is affected?

Component, Accessibility

### Description

Putting focusable components - such as a `md-checkbox` - inside a `md-menu-item` and subsequently pressing 'TAB' when focused on said menu-item, correctly delegates focus to its inner checkbox, but also deactivates the parent `md-menu-item` - e.g sets `tabIndex = 0`.
Further navigation - e.g pressing `ArrowUp` - which is handled through a call to the `menu.listController.handleKeydown` function, will conclude that no items are active and therefore the call to `activatePreviousItem` with `activeItemRecord` being a `null`-value results in a call to `activateLastItem`.
The last list item gets focus, instead of the item above.

The culprit is the `menu.typeaheadController` whose `onKeydown`-function is called through the capturing event listener `menu.captureKeydown`. As `isTypingAhead == false` `beginTypeahead` gets called.

There are checks for whether or not the given key is a navigable key - upon which it should return - but only for arrow keys, space, enter and escape - hardcoded. Therefore shift and tabkeys begin typeahead's which sets `tabIndex = -1` for the `lastActiveItemRecord` (the first menu-item with `tabIndex == 0`);

### Reproduction

The below code (partially from the documentation) for a menu and activating button should display the described behaviour, when tabbing while focus is on the 'banana' `md-menu-item`, and then trying to navigate afterwards

```
Set with idref


Apple



Apple 2



Banana




Apple 3



Apple 4



Cucumber

```

### Workaround

Including more checks for which keys are navigable/non-typeahead;
Could just add shiftLeft and tab to the long if-statement, or maybe provide an extension point like the `listController`'s `isNavigableKey` function :) ;

### Is this a regression?

No or unsure. This never worked, or I haven't tried before.

### Affected versions

All (i imagine)

### Browser/OS/Node environment

unrelated i believe

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start at the menu typeaheadController.onKeydown path invoked by menu.captureKeydown, then trace listController.handleKeydown, activatePreviousItem, and beginTypeahead. Add a regression test for Tab navigation from a menu item containing a checkbox, and verify that subsequent ArrowUp navigation focuses the preceding item rather than the last item.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
accessibility, frontend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.