diff --git a/CHANGELOG.md b/CHANGELOG.md index caaf2e864..7b7892673 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ CHANGELOG ## HEAD (Unreleased) * @OwenEdwards added ClickableComponent. Fixed keyboard operation of buttons ([view](https://github.com/videojs/video.js/pull/3032)) +* @OwenEdwards Fixed menu keyboard access and ARIA labeling for screen readers ([view](https://github.com/videojs/video.js/pull/3033)) -------------------- diff --git a/src/js/control-bar/control-bar.js b/src/js/control-bar/control-bar.js index da07a80fa..df0e4c433 100644 --- a/src/js/control-bar/control-bar.js +++ b/src/js/control-bar/control-bar.js @@ -38,6 +38,8 @@ class ControlBar extends Component { createEl() { return super.createEl('div', { className: 'vjs-control-bar' + }, { + 'role': 'group' // The control bar is a group, so it can contain menuitems }); } } diff --git a/src/js/control-bar/text-track-controls/caption-settings-menu-item.js b/src/js/control-bar/text-track-controls/caption-settings-menu-item.js index cb43bf82c..712543f20 100644 --- a/src/js/control-bar/text-track-controls/caption-settings-menu-item.js +++ b/src/js/control-bar/text-track-controls/caption-settings-menu-item.js @@ -19,12 +19,17 @@ import Component from '../../component.js'; 'kind': options['kind'], 'player': player, 'label': options['kind'] + ' settings', + 'selectable': false, 'default': false, mode: 'disabled' }; + // CaptionSettingsMenuItem has no concept of 'selected' + options['selectable'] = false; + super(player, options); this.addClass('vjs-texttrack-settings'); + this.controlText(', opens ' + options['kind'] + ' settings dialog'); } /** @@ -34,6 +39,7 @@ import Component from '../../component.js'; */ handleClick() { this.player().getChild('textTrackSettings').show(); + this.player().getChild('textTrackSettings').el_.focus(); } } diff --git a/src/js/control-bar/text-track-controls/off-text-track-menu-item.js b/src/js/control-bar/text-track-controls/off-text-track-menu-item.js index 8238c42ab..9cc124b9d 100644 --- a/src/js/control-bar/text-track-controls/off-text-track-menu-item.js +++ b/src/js/control-bar/text-track-controls/off-text-track-menu-item.js @@ -25,6 +25,9 @@ class OffTextTrackMenuItem extends TextTrackMenuItem { 'mode': 'disabled' }; + // MenuItem is selectable + options['selectable'] = true; + super(player, options); this.selected(true); } diff --git a/src/js/control-bar/text-track-controls/text-track-button.js b/src/js/control-bar/text-track-controls/text-track-button.js index 07dd39bcd..514a372a7 100644 --- a/src/js/control-bar/text-track-controls/text-track-button.js +++ b/src/js/control-bar/text-track-controls/text-track-button.js @@ -57,6 +57,8 @@ class TextTrackButton extends MenuButton { // only add tracks that are of the appropriate kind and have a label if (track['kind'] === this.kind_) { items.push(new TextTrackMenuItem(this.player_, { + // MenuItem is selectable + 'selectable': true, 'track': track })); } diff --git a/src/js/control-bar/text-track-controls/text-track-menu-item.js b/src/js/control-bar/text-track-controls/text-track-menu-item.js index 2fb10ba61..8b4c73bda 100644 --- a/src/js/control-bar/text-track-controls/text-track-menu-item.js +++ b/src/js/control-bar/text-track-controls/text-track-menu-item.js @@ -24,6 +24,7 @@ class TextTrackMenuItem extends MenuItem { // Modify options for parent MenuItem class's init. options['label'] = track['label'] || track['language'] || 'Unknown'; options['selected'] = track['default'] || track['mode'] === 'showing'; + super(player, options); this.track = track; diff --git a/src/js/menu/menu-button.js b/src/js/menu/menu-button.js index dc31af0e2..faa4a27a7 100644 --- a/src/js/menu/menu-button.js +++ b/src/js/menu/menu-button.js @@ -23,8 +23,9 @@ class MenuButton extends ClickableComponent { this.update(); - this.on('keydown', this.handleKeyPress); this.el_.setAttribute('aria-haspopup', true); + this.el_.setAttribute('role', 'menuitem'); + this.on('keydown', this.handleSubmenuKeyPress); } /** @@ -49,6 +50,7 @@ class MenuButton extends ClickableComponent { * @private */ this.buttonPressed_ = false; + this.el_.setAttribute('aria-expanded', false); if (this.items && this.items.length === 0) { this.hide(); @@ -125,27 +127,6 @@ class MenuButton extends ClickableComponent { return `vjs-menu-button ${menuButtonClass} ${super.buildCSSClass()}`; } - /** - * Focus - Add keyboard functionality to element - * This function is not needed anymore. Instead, the - * keyboard functionality is handled by - * treating the button as triggering a submenu. - * When the button is pressed, the submenu - * appears. Pressing the button again makes - * the submenu disappear. - * - * @method handleFocus - */ - handleFocus() {} - - /** - * Can't turn off list display that we turned - * on with focus, because list would go away. - * - * @method handleBlur - */ - handleBlur() {} - /** * When you click the button it adds focus, which * will show the menu indefinitely. @@ -170,25 +151,48 @@ class MenuButton extends ClickableComponent { /** * Handle key press on menu * - * @param {Object} Key press event + * @param {Object} event Key press event * @method handleKeyPress */ handleKeyPress(event) { - // Check for space bar (32) or enter (13) keys - if (event.which === 32 || event.which === 13) { - if (this.buttonPressed_){ + // Escape (27) key or Tab (9) key unpress the 'button' + if (event.which === 27 || event.which === 9) { + if (this.buttonPressed_) { this.unpressButton(); - } else { + } + // Don't preventDefault for Tab key - we still want to lose focus + if (event.which !== 9) { + event.preventDefault(); + } + // Up (38) key or Down (40) key press the 'button' + } else if (event.which === 38 || event.which === 40) { + if (!this.buttonPressed_) { this.pressButton(); + event.preventDefault(); } - event.preventDefault(); - // Check for escape (27) key - } else if (event.which === 27){ + } else { + super.handleKeyPress(event); + } + } + + /** + * Handle key press on submenu + * + * @param {Object} event Key press event + * @method handleSubmenuKeyPress + */ + handleSubmenuKeyPress(event) { + + // Escape (27) key or Tab (9) key unpress the 'button' + if (event.which === 27 || event.which === 9){ if (this.buttonPressed_){ this.unpressButton(); } - event.preventDefault(); + // Don't preventDefault for Tab key - we still want to lose focus + if (event.which !== 9) { + event.preventDefault(); + } } } @@ -200,10 +204,8 @@ class MenuButton extends ClickableComponent { pressButton() { this.buttonPressed_ = true; this.menu.lockShowing(); - this.el_.setAttribute('aria-pressed', true); - if (this.items && this.items.length > 0) { - this.items[0].el().focus(); // set the focus to the title of the submenu - } + this.el_.setAttribute('aria-expanded', true); + this.menu.focus(); // set the focus into the submenu } /** @@ -214,7 +216,8 @@ class MenuButton extends ClickableComponent { unpressButton() { this.buttonPressed_ = false; this.menu.unlockShowing(); - this.el_.setAttribute('aria-pressed', false); + this.el_.setAttribute('aria-expanded', false); + this.el_.focus(); // Set focus back to this menu button } } diff --git a/src/js/menu/menu-item.js b/src/js/menu/menu-item.js index aed29678b..555abe0a0 100644 --- a/src/js/menu/menu-item.js +++ b/src/js/menu/menu-item.js @@ -17,7 +17,18 @@ class MenuItem extends ClickableComponent { constructor(player, options) { super(player, options); + + this.selectable = options['selectable']; + this.selected(options['selected']); + + if (this.selectable) { + // TODO: May need to be either menuitemcheckbox or menuitemradio, + // and may need logical grouping of menu items. + this.el_.setAttribute('role', 'menuitemcheckbox'); + } else { + this.el_.setAttribute('role', 'menuitem'); + } } /** @@ -31,7 +42,8 @@ class MenuItem extends ClickableComponent { createEl(type, props, attrs) { return super.createEl('li', assign({ className: 'vjs-menu-item', - innerHTML: this.localize(this.options_['label']) + innerHTML: this.localize(this.options_['label']), + tabIndex: -1 }, props), attrs); } @@ -51,15 +63,22 @@ class MenuItem extends ClickableComponent { * @method selected */ selected(selected) { - if (selected) { - this.addClass('vjs-selected'); - this.el_.setAttribute('aria-selected',true); - } else { - this.removeClass('vjs-selected'); - this.el_.setAttribute('aria-selected',false); + if (this.selectable) { + if (selected) { + this.addClass('vjs-selected'); + this.el_.setAttribute('aria-checked',true); + // aria-checked isn't fully supported by browsers/screen readers, + // so indicate selected state to screen reader in the control text. + this.controlText(', selected'); + } else { + this.removeClass('vjs-selected'); + this.el_.setAttribute('aria-checked',false); + // Indicate un-selected state to screen reader + // Note that a space clears out the selected state text + this.controlText(' '); + } } } - } Component.registerComponent('MenuItem', MenuItem); diff --git a/src/js/menu/menu.js b/src/js/menu/menu.js index 7efaa060b..1fa9923a2 100644 --- a/src/js/menu/menu.js +++ b/src/js/menu/menu.js @@ -15,6 +15,14 @@ import * as Events from '../utils/events.js'; */ class Menu extends Component { + constructor (player, options) { + super(player, options); + + this.focusedChild_ = -1; + + this.on('keydown', this.handleKeyPress); + } + /** * Add a menu item to the menu * @@ -25,6 +33,7 @@ class Menu extends Component { this.addChild(component); component.on('click', Fn.bind(this, function(){ this.unlockShowing(); + //TODO: Need to set keyboard focus back to the menuButton })); } @@ -39,10 +48,12 @@ class Menu extends Component { this.contentEl_ = Dom.createEl(contentElType, { className: 'vjs-menu-content' }); + this.contentEl_.setAttribute('role', 'menu'); var el = super.createEl('div', { append: this.contentEl_, className: 'vjs-menu' }); + el.setAttribute('role', 'presentation'); el.appendChild(this.contentEl_); // Prevent clicks from bubbling up. Needed for Menu Buttons, @@ -54,6 +65,72 @@ class Menu extends Component { return el; } + + /** + * Handle key press for menu + * + * @param {Object} event Event object + * @method handleKeyPress + */ + handleKeyPress (event) { + if (event.which === 37 || event.which === 40) { // Left and Down Arrows + event.preventDefault(); + this.stepForward(); + } else if (event.which === 38 || event.which === 39) { // Up and Right Arrows + event.preventDefault(); + this.stepBack(); + } + } + + /** + * Move to next (lower) menu item for keyboard users + * + * @method stepForward + */ + stepForward () { + let stepChild = 0; + + if (this.focusedChild_ !== undefined) { + stepChild = this.focusedChild_ + 1; + } + this.focus(stepChild); + } + + /** + * Move to previous (higher) menu item for keyboard users + * + * @method stepBack + */ + stepBack () { + let stepChild = 0; + + if (this.focusedChild_ !== undefined) { + stepChild = this.focusedChild_ - 1; + } + this.focus(stepChild); + } + + /** + * Set focus on a menu item in the menu + * + * @param {Object|String} item Index of child item set focus on + * @method focus + */ + focus (item = 0) { + let children = this.children(); + + if (children.length > 0) { + if (item < 0) { + item = 0; + } else if (item >= children.length) { + item = children.length - 1; + } + + this.focusedChild_ = item; + + children[item].el_.focus(); + } + } } Component.registerComponent('Menu', Menu);