feat(list): Add arrow key a11y support. #2871
Conversation
williamernest
force-pushed
the
feat/list/new-component
branch
2 times, most recently
from
9785d07
to
b7d0370
Compare
Codecov Report
@@ Coverage Diff @@
## master #2871 +/- ##
=========================================
+ Coverage 98.25% 98.3% +0.05%
=========================================
Files 98 101 +3
Lines 4231 4359 +128
Branches 540 563 +23
=========================================
+ Hits 4157 4285 +128
Misses 74 74
Continue to review full report at Codecov.
|
williamernest
force-pushed
the
feat/list/new-component
branch
from
d156819
to
adc1ece
Compare
There was a problem hiding this comment.
-
missing tabindex='0' on this line https://github.com/material-components/material-components-web/pull/2871/files#diff-c054c03788286df0d6a7acf64dcd3945R966
-
All demos are not orientation=vertical, which is more intuitive to me. Do you agree they should be vertical too?
-
The hero list has 2 meta data icons. I don't think that is in the spec/not recommended (I'm just guessing).
packages/mdc-list/foundation.js
Outdated
| * @private | ||
| */ | ||
| preventDefaultEvent_(evt) { | ||
| const tagName = ('' + evt.target.tagName).toLowerCase(); |
There was a problem hiding this comment.
you could do
`${evt.target.tagName}`.toLowerCase()
packages/mdc-list/foundation.js
Outdated
| } | ||
|
|
||
| focusFirstElement() { | ||
| this.adapter_.focusItemAtIndex(0); |
There was a problem hiding this comment.
do you need a check here to make sure there this.adapter_.getListItemCount() > 0 ?
packages/mdc-list/foundation.js
Outdated
| * @private | ||
| */ | ||
| getListItem_(target) { | ||
| while (!target.classList.contains('mdc-list-item')) { |
There was a problem hiding this comment.
I think 'mdc-list-item' should be in strings
packages/mdc-list/index.js
Outdated
| import {strings} from './constants'; | ||
|
|
||
| /** | ||
| * @extends MDCComponent<!MDCistFoundation> |
There was a problem hiding this comment.
MDCistFoundation => MDCListFoundation
| const direction = this.root_.getAttribute(strings.ARIA_ORIENTATION); | ||
| this.vertical = direction === strings.ARIA_ORIENTATION_VERTICAL; | ||
|
|
||
| // List items need to have at least tabindex=-1 to be focusable. |
There was a problem hiding this comment.
No, tabindex=-1. li elements cannot be focused unless they have a tabIndex value. If it's 0, then they're in the tab order. If it's -1, they're not in the tab order, but they are focusable.
packages/mdc-list/README.md
Outdated
|
|
||
| As the user navigates through the list, any `button` or `a` elements within the list will receive `tabindex="-1"` | ||
| when the list item is not focused. When the list item receives focus, the child `button` and `a` elements will | ||
| receive `tabIndex="0"`. This allows for the user to tab through a list items elements and then tab to the |
There was a problem hiding this comment.
through list item elements and...
packages/mdc-list/README.md
Outdated
| --- | --- | ||
| `ArrowUp` | When the list is in a vertical orientation, it will cause the previous list item to receive focus. | ||
| `ArrowDown` | When the list is in a vertical orientation, it will cause the next list item to receive focus. | ||
| `ArrowLeft` | When the list is in a horizontal orientation, it will cause the previous list item to receive focus. |
There was a problem hiding this comment.
When the list is in a horizontal orientation (default), ...
packages/mdc-list/README.md
Outdated
| `setWrapFocus(value: Boolean) => void` | Sets the list to allow the up arrow on the first element to focus the last element of the list and vice versa. | ||
| `setVerticalOrientation(value: Boolean) => void` | Sets the list to an orientation causing the keys used for navigation to change. `true` results in the Up/Down arrow keys being used. `false` results in the Left/Right arrow keys being used. | ||
| `handleFocusIn(evt: Event) => void` | Handles the changing of `tabindex` to `0` for all `button` and `a` elements when a list item receives focus. | ||
| `handleFocusOut(evt: Event) => void` | Handles the changing of `tabindex` to `-1` for all `button` and `a` elements when a list item receives focus. |
There was a problem hiding this comment.
when a list item loses focus (blur)?
|
I am investigating a bug where pressing the up/down keys while a list is expanded will make the selected list item skip an element of the list. Is anyone familiar with this? Thank you in advance. |
|
@patbou02 We came across this same issue with the arrow keys skipping every other element in our list. We found that the MDC drawer and MDC list are both trying to control the arrow key functions, so we removed the |
fixes: #2850