feat(a11y): manager for list keyboard events #1599
Conversation
googlebot
added
the
cla: yes
kara
force-pushed
the
keyboard-toolkit
branch
from
54d2359
to
19499c0
Compare
| disabled: boolean; | ||
| } | ||
|
|
||
| export class ListKeyManager { |
| import {EventEmitter, Output, QueryList} from '@angular/core'; | ||
| import {UP_ARROW, DOWN_ARROW, TAB} from '../core'; | ||
|
|
||
| export interface MdFocusable { |
| export class ListKeyManager { | ||
| private _focusedItemIndex: number; | ||
|
|
||
| @Output() tabOut: EventEmitter<null> = new EventEmitter(); |
There was a problem hiding this comment.
Add field description.
Also, should it be EventEmitter<void>?
| this._focusedItemIndex = value; | ||
| } | ||
|
|
||
| // TODO(kara): update this when (keydown.downArrow) testability is fixed |
There was a problem hiding this comment.
I don't this this TODO applies any more
| disabled: boolean; | ||
| } | ||
|
|
||
| export class ListKeyManager { |
There was a problem hiding this comment.
We should have unit tests for just this class (with some fake MdFocusable items)
| import {EventEmitter, Output, QueryList} from '@angular/core'; | ||
| import {UP_ARROW, DOWN_ARROW, TAB} from '../core'; | ||
|
|
||
| export interface MdFocusable { |
| this._focusedItemIndex = value; | ||
| } | ||
|
|
||
| // TODO(kara): update this when (keydown.downArrow) testability is fixed |
| @@ -67,61 +72,16 @@ export class MdMenu { | |||
| */ | |||
| _focusFirstItem() { | |||
| this.items.first.focus(); | |||
| this._keyManager.focusedItemIndex = 0; | |||
There was a problem hiding this comment.
Add comment like
// The menu always opens with the first item focused.| export class ListKeyManager { | ||
| private _focusedItemIndex: number; | ||
|
|
||
| @Output() tabOut: EventEmitter<null> = new EventEmitter(); |
| private _updateFocusedItemIndex(delta: number, items: MdFocusable[]) { | ||
| // when focus would leave menu, wrap to beginning or end | ||
| this._focusedItemIndex = (this._focusedItemIndex + delta + items.length) | ||
| % items.length; |
There was a problem hiding this comment.
nit: it's a little more readable to break at the highest semantic level, i.e.:
this._focusedItemIndex =
(this._focusedItemIndex + delta + items.length) % items.length;
kara
force-pushed
the
keyboard-toolkit
branch
4 times, most recently
from
b32a186
to
415b5a4
Compare
| import {ListKeyManager, MdFocusable} from './list-key-manager'; | ||
| import {DOWN_ARROW, UP_ARROW, TAB} from '../keyboard/keycodes'; | ||
|
|
||
| class MockFocusable { |
There was a problem hiding this comment.
nit: technically it's a FakeFocusable
| new MockFocusable() | ||
| ]; | ||
|
|
||
| itemList.toArray = () => { return items; }; |
There was a problem hiding this comment.
itemList.toArray = () => items;| * This event is emitted any time the TAB key is pressed, so components can react | ||
| * when focus is shifted off of the list. | ||
| */ | ||
| @Output() tabOut: EventEmitter<null> = new EventEmitter(); |
There was a problem hiding this comment.
Should this be void instead of null?
There was a problem hiding this comment.
I can change. edit: we discussed in person
kara
force-pushed
the
keyboard-toolkit
branch
from
415b5a4
to
c4a6015
Compare
|
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. |
This PR adds a reusable keyboard event manager for selectable lists (e.g. menu, select, and nav list). Existing e2e tests should cover this functionality.
r: @jelbourn