From 31ffce4b05ab5ae66139235897659d91bcc2c266 Mon Sep 17 00:00:00 2001 From: jmclaus Date: Thu, 30 Jan 2014 13:05:20 -0500 Subject: [PATCH] Keyboard events and ARIA markup added to speed control. Replaced anonymous event handlers by named functions. Menu stays open on mouseleave when a speed entry has focus. In that case, the menu can be closed by clicking anywhere outside of it. [BLD-402, BLD-363] --- .../js/spec/video/video_speed_control_spec.js | 247 ++++++++++------ .../js/src/video/08_video_speed_control.js | 273 ++++++++++-------- lms/templates/video.html | 2 +- 3 files changed, 317 insertions(+), 205 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js index 2140b6f315..702b185fc3 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js @@ -53,12 +53,6 @@ 'aria-disabled': 'false' }); }); - - it('bind to change video speed link', function () { - expect($('.video_speeds a')).toHandleWith( - 'click', state.videoSpeedControl.changeVideoSpeed - ); - }); }); describe('when running on touch based device', function () { @@ -73,71 +67,184 @@ }); describe('when running on non-touch based device', function () { + var speedControl, speedEntries, + KEY = $.ui.keyCode, + + keyPressEvent = function(key) { + return $.Event('keydown', {keyCode: key}); + }, + + tabBackPressEvent = function() { + return $.Event('keydown', + {keyCode: KEY.TAB, shiftKey: true}); + }, + + tabForwardPressEvent = function() { + return $.Event('keydown', + {keyCode: KEY.TAB, shiftKey: false}); + }, + + // Get previous element in array or cyles back to the last + // if it is the first. + previousSpeed = function(index) { + return speedEntries.eq(index < 1 ? + speedEntries.length - 1 : + index - 1); + }, + + // Get next element in array or cyles back to the first if + // it is the last. + nextSpeed = function(index) { + return speedEntries.eq(index >= speedEntries.length-1 ? + 0 : + index + 1); + }; + beforeEach(function () { state = jasmine.initializePlayer(); + speedControl = $('div.speeds'); + speedEntries = speedControl.children('a'); + spyOn($.fn, 'focus').andCallThrough(); }); - it('open the speed toggle on hover', function () { - $('.speeds').mouseenter(); - expect($('.speeds')).toHaveClass('open'); - - $('.speeds').mouseleave(); - expect($('.speeds')).not.toHaveClass('open'); + it('open/close the speed menu on mouseenter/mouseleave', + function () { + speedControl.mouseenter(); + expect(speedControl).toHaveClass('open'); + speedControl.mouseleave(); + expect(speedControl).not.toHaveClass('open'); }); - it('close the speed toggle on mouse out', function () { - $('.speeds').mouseenter().mouseleave(); - - expect($('.speeds')).not.toHaveClass('open'); + it('do not close the speed menu on mouseleave if a speed ' + + 'entry has focus', function () { + // Open speed meenu. Focus is on last speed entry. + speedControl.trigger(keyPressEvent(KEY.ENTER)); + speedControl.mouseenter().mouseleave(); + expect(speedControl).toHaveClass('open'); }); - it('close the speed toggle on click', function () { - $('.speeds').mouseenter().click(); - - expect($('.speeds')).not.toHaveClass('open'); + it('close the speed menu on click', function () { + speedControl.mouseenter().click(); + expect(speedControl).not.toHaveClass('open'); }); - // Tabbing depends on the following order: - // 1. Play anchor - // 2. Speed anchor - // 3. A number of speed entry anchors - // 4. Volume anchor - // If another focusable element is inserted or if the order is - // changed, things will malfunction as a flag, - // state.previousFocus, is set in the 1,3,4 elements and is - // used to determine the behavior of foucus() and blur() for - // the speed anchor. - it( - 'checks for a certain order in focusable elements in ' + - 'video controls', - function () - { - var foundFirst = false, - playIndex, speedIndex, firstSpeedEntry, lastSpeedEntry, - volumeIndex; + it('close the speed menu on outside click', function () { + speedControl.trigger(keyPressEvent(KEY.ENTER)); + $(window).click(); + expect(speedControl).not.toHaveClass('open'); + }); - $('.video-controls').find('a, :focusable').each( - function (index) - { - if ($(this).hasClass('video_control')) { - playIndex = index; - } else if ($(this).parent().hasClass('speeds')) { - speedIndex = index; - } else if ($(this).hasClass('speed_link')) { - if (!foundFirst) { - firstSpeedEntry = index; - foundFirst = true; - } + it('open the speed menu on ENTER keydown', function () { + speedControl.trigger(keyPressEvent(KEY.ENTER)); + expect(speedControl).toHaveClass('open'); + expect(speedEntries.last().focus).toHaveBeenCalled(); + }); - lastSpeedEntry = index; - } else if ($(this).parent().hasClass('volume')) { - volumeIndex = index; - } - }); + it('open the speed menu on SPACE keydown', function () { + speedControl.trigger(keyPressEvent(KEY.SPACE)); + expect(speedControl).toHaveClass('open'); + expect(speedEntries.last().focus).toHaveBeenCalled(); + }); - expect(playIndex+1).toEqual(speedIndex); - expect(speedIndex+1).toEqual(firstSpeedEntry); - expect(lastSpeedEntry+1).toEqual(volumeIndex); + it('open the speed menu on UP keydown', function () { + speedControl.trigger(keyPressEvent(KEY.UP)); + expect(speedControl).toHaveClass('open'); + expect(speedEntries.last().focus).toHaveBeenCalled(); + }); + + it('close the speed menu on ESCAPE keydown', function () { + speedControl.trigger(keyPressEvent(KEY.ESCAPE)); + expect(speedControl).not.toHaveClass('open'); + }); + + it('UP and DOWN keydown function as expected on speed entries', + function () { + // Iterate through list in both directions and check if + // things wrap up correctly. + var lastEntry = speedEntries.length-1, i; + + // First open menu + speedControl.trigger(keyPressEvent(KEY.UP)); + + // Iterate with UP key until we have looped. + for (i = lastEntry; i >= 0; i--) { + speedEntries.eq(i).trigger(keyPressEvent(KEY.UP)); + } + + // Iterate with DOWN key until we have looped. + for (i = 0; i <= lastEntry; i++) { + speedEntries.eq(i).trigger(keyPressEvent(KEY.DOWN)); + } + // Test if each element has been called twice. + expect($.fn.focus.calls.length) + .toEqual(2*speedEntries.length); + }); + + it('ESC keydown on speed entry closes menu', function () { + // First open menu. Focus is on last speed entry. + speedControl.trigger(keyPressEvent(KEY.UP)); + speedEntries.last().trigger(keyPressEvent(KEY.ESCAPE)); + + // Menu is closed and focus has been returned to speed + // control. + expect(speedControl).not.toHaveClass('open'); + expect(speedControl.focus).toHaveBeenCalled(); + }); + + it('ENTER keydown on speed entry selects speed and closes menu', + function () { + // First open menu. + speedControl.trigger(keyPressEvent(KEY.UP)); + // Focus on 1.50x speed + speedEntries.eq(1).focus(); + speedEntries.eq(1).trigger(keyPressEvent(KEY.ENTER)); + + // Menu is closed, focus has been returned to speed + // control and video speed is 1.50x. + expect(speedControl.focus).toHaveBeenCalled(); + expect($('.video_speeds li[data-speed="1.50"]')) + .toHaveClass('active'); + expect($('.speeds p.active')).toHaveHtml('1.50x'); + }); + + it('SPACE keydown on speed entry selects speed and closes menu', + function () { + // First open menu. + speedControl.trigger(keyPressEvent(KEY.UP)); + // Focus on 1.50x speed + speedEntries.eq(1).focus(); + speedEntries.eq(1).trigger(keyPressEvent(KEY.SPACE)); + + // Menu is closed, focus has been returned to speed + // control and video speed is 1.50x. + expect(speedControl.focus).toHaveBeenCalled(); + expect($('.video_speeds li[data-speed="1.50"]')) + .toHaveClass('active'); + expect($('.speeds p.active')).toHaveHtml('1.50x'); + }); + + it('TAB + SHIFT keydown on speed entry closes menu and gives ' + + 'focus to Play/Pause control', function () { + // First open menu. Focus is on last speed entry. + speedControl.trigger(keyPressEvent(KEY.UP)); + speedEntries.last().trigger(tabBackPressEvent()); + + // Menu is closed and focus has been given to Play/Pause + // control. + expect(state.videoControl.playPauseEl.focus) + .toHaveBeenCalled(); + }); + + it('TAB keydown on speed entry closes menu and gives focus ' + + 'to Volume control', function () { + // First open menu. Focus is on last speed entry. + speedControl.trigger(keyPressEvent(KEY.UP)); + speedEntries.last().trigger(tabForwardPressEvent()); + + // Menu is closed and focus has been given to Volume + // control. + expect(state.videoVolumeControl.buttonEl.focus) + .toHaveBeenCalled(); }); }); }); @@ -163,30 +270,6 @@ expect(state.videoSpeedControl.currentSpeed).toEqual(0.75); }); }); - - describe( - 'make sure the speed control gets the focus afterwards', - function () - { - var anchor; - - beforeEach(function () { - state = jasmine.initializePlayer(); - anchor= $('.speeds > a').first(); - state.videoSpeedControl.setSpeed(1.0); - spyOnEvent(anchor, 'focus'); - }); - - it('when the speed is the same', function () { - $('li[data-speed="1.0"] a').click(); - expect('focus').toHaveBeenTriggeredOn(anchor); - }); - - it('when the speed is not the same', function () { - $('li[data-speed="0.75"] a').click(); - expect('focus').toHaveBeenTriggeredOn(anchor); - }); - }); }); describe('onSpeedChange', function () { diff --git a/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js b/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js index c8ceeb1770..731ef1e987 100644 --- a/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js +++ b/common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js @@ -117,6 +117,142 @@ function () { state.el.find('div.speeds').hide(); } + // Get previous element in array or cyles back to the last if it is the + // first. + function _previousSpeedLink(speedLinks, index) { + return $(speedLinks.eq(index < 1 ? speedLinks.length - 1 : index - 1)); + } + + // Get next element in array or cyles back to the first if it is the last. + function _nextSpeedLink(speedLinks, index) { + return $(speedLinks.eq(index >= speedLinks.length - 1 ? 0 : index + 1)); + } + + function _speedLinksFocused(state) { + var speedLinks = state.videoSpeedControl.videoSpeedsEl + .find('a.speed_link'); + return speedLinks.is(':focus'); + } + + function _openMenu(state) { + // When speed entries have focus, the menu stays open on + // mouseleave. A clickHandler is added to the window + // element to have clicks close the menu when they happen + // outside of it. + $(window).on('click.speedMenu', _clickHandler.bind(state)); + state.videoSpeedControl.el.addClass('open'); + } + + function _closeMenu(state) { + // Remove the previously added clickHandler from window element. + $(window).off('click.speedMenu'); + state.videoSpeedControl.el.removeClass('open'); + } + + // Various event handlers. They all return false to stop propagation and + // prevent default behavior. + function _clickHandler(event) { + var target = $(event.currentTarget); + + this.videoSpeedControl.el.removeClass('open'); + if (target.is('a.speed_link')) { + this.videoSpeedControl.changeVideoSpeed.call(this, event); + } + + return false; + } + + // We do not use _openMenu and _closeMenu in the following two handlers + // because we do not want to add an unnecessary clickHandler to the window + // element. + function _mouseEnterHandler(event) { + this.videoSpeedControl.el.addClass('open'); + + return false; + } + + function _mouseLeaveHandler(event) { + // Only close the menu is no speed entry has focus. + if (!_speedLinksFocused(this)) { + this.videoSpeedControl.el.removeClass('open'); + } +          + return false; + } + + function _keyDownHandler(event) { + var KEY = $.ui.keyCode, + keyCode = event.keyCode, + target = $(event.currentTarget), + speedButtonLink = this.videoSpeedControl.el.children('a'), + speedLinks = this.videoSpeedControl.videoSpeedsEl + .find('a.speed_link'), + index; + + if (target.is('a.speed_link')) { + + index = target.parent().index(); + + switch (keyCode) { + // Scroll up menu, wrapping at the top. Keep menu open. + case KEY.UP: + _previousSpeedLink(speedLinks, index).focus(); + break; + // Scroll down menu, wrapping at the bottom. Keep menu + // open. + case KEY.DOWN: + _nextSpeedLink(speedLinks, index).focus(); + break; + // Close menu. + case KEY.TAB: + _closeMenu(this); + // Set focus to previous menu button in menu bar + // (Play/Pause button) + if (event.shiftKey) { + this.videoControl.playPauseEl.focus(); + } + // Set focus to next menu button in menu bar + // (Volume button) + else { + this.videoVolumeControl.buttonEl.focus(); + } + break; + // Close menu, give focus to speed control and change + // speed. + case KEY.ENTER: + case KEY.SPACE: + _closeMenu(this); + speedButtonLink.focus(); + this.videoSpeedControl.changeVideoSpeed.call(this, event); + break; + // Close menu and give focus to speed control. + case KEY.ESCAPE: + _closeMenu(this); + speedButtonLink.focus(); + break; + } + return false; + } + else { + switch(keyCode) { + // Open menu and focus on last element of list above it. + case KEY.ENTER: + case KEY.SPACE: + case KEY.UP: + _openMenu(this); + speedLinks.last().focus(); + break; + // Close menu. + case KEY.ESCAPE: + _closeMenu(this); + break; + } + // We do not stop propagation and default behavior on a TAB + // keypress. + return event.keyCode === KEY.TAB; + } +    } + /** * @desc Bind any necessary function callbacks to DOM events (click, * mousemove, etc.). @@ -133,125 +269,21 @@ function () { * @returns {undefined} */ function _bindHandlers(state) { - var speedLinks; + var speedButton = state.videoSpeedControl.el, + videoSpeeds = state.videoSpeedControl.videoSpeedsEl; - state.videoSpeedControl.videoSpeedsEl.find('a') - .on('click', state.videoSpeedControl.changeVideoSpeed); + // Attach various events handlers to the speed menu button. + speedButton.on({ + 'mouseenter': _mouseEnterHandler.bind(state), + 'mouseleave': _mouseLeaveHandler.bind(state), + 'click': _clickHandler.bind(state), + 'keydown': _keyDownHandler.bind(state) + }); - if (state.isTouch) { - state.videoSpeedControl.el.on('click', function (event) { - // So that you can't highlight this control via a drag - // operation, we disable the default browser actions on a - // click event. - event.preventDefault(); - - state.videoSpeedControl.el.toggleClass('open'); - }); - } else { - state.videoSpeedControl.el - .on('mouseenter', function () { - state.videoSpeedControl.el.addClass('open'); - }) - .on('mouseleave', function () { - state.videoSpeedControl.el.removeClass('open'); - }) - .on('click', function (event) { - // So that you can't highlight this control via a drag - // operation, we disable the default browser actions on a - // click event. - event.preventDefault(); - - state.videoSpeedControl.el.removeClass('open'); - }); - - // ****************************** - // The tabbing will cycle through the elements in the following - // order: - // 1. Play control - // 2. Speed control - // 3. Fastest speed called firstSpeed - // 4. Intermediary speed called otherSpeed - // 5. Slowest speed called lastSpeed - // 6. Volume control - // This field will keep track of where the focus is coming from. - state.previousFocus = ''; - - // ****************************** - // Attach 'focus', and 'blur' events to the speed control which - // either brings up the speed dialog with individual speed entries, - // or closes it. - state.videoSpeedControl.el.children('a') - .on('focus', function () { - // If the focus is coming from the first speed entry - // (tabbing backwards) or last speed entry (tabbing forward) - // hide the speed entries dialog. - if (state.previousFocus === 'firstSpeed' || - state.previousFocus === 'lastSpeed') { - state.videoSpeedControl.el.removeClass('open'); - } - }) - .on('blur', function () { - // When the focus leaves this element, the speed entries - // dialog will be shown. - - // If we are tabbing forward (previous focus is play - // control), we open the dialog and set focus on the first - // speed entry. - if (state.previousFocus === 'playPause') { - state.videoSpeedControl.el.addClass('open'); - state.videoSpeedControl.videoSpeedsEl - .find('a.speed_link:first') - .focus(); - } - - // If we are tabbing backwards (previous focus is volume - // control), we open the dialog and set focus on the - // last speed entry. - if (state.previousFocus === 'volume') { - state.videoSpeedControl.el.addClass('open'); - state.videoSpeedControl.videoSpeedsEl - .find('a.speed_link:last') - .focus(); - } - - }); - - // ****************************** - // Attach 'blur' event to elements which represent individual speed - // entries and use it to track the origin of the focus. - speedLinks = state.videoSpeedControl.videoSpeedsEl - .find('a.speed_link'); - - speedLinks.first().on('blur', function () { - // The previous focus is a speed entry (we are tabbing - // backwards), the dialog will close, set focus on the speed - // control and track the focus on first speed. - if (state.previousFocus === 'otherSpeed') { - state.previousFocus = 'firstSpeed'; - state.videoSpeedControl.el.children('a').focus(); - } - }); - - // Track the focus on intermediary speeds. - speedLinks - .filter(function (index) { - return index === 1 || index === 2; - }) - .on('blur', function () { - state.previousFocus = 'otherSpeed'; - }); - - speedLinks.last().on('blur', function () { - // The previous focus is a speed entry (we are tabbing forward), - // the dialog will close, set focus on the speed control and - // track the focus on last speed. - if (state.previousFocus === 'otherSpeed') { - state.previousFocus = 'lastSpeed'; - state.videoSpeedControl.el.children('a').focus(); - } - }); - - } + // Attach click and keydown event handlers to the individual speed + // entries. + videoSpeeds.on('click', 'a.speed_link', _clickHandler.bind(state)) + .on('keydown', 'a.speed_link', _keyDownHandler.bind(state)); } // *************************************************************** @@ -289,9 +321,6 @@ function () { this.videoSpeedControl.currentSpeed ); } - // When a speed entry has been selected, we want the speed control to - // regain focus. - parentEl.parent().siblings('a').focus(); } function reRender(params) { @@ -304,9 +333,9 @@ function () { $.each(this.videoSpeedControl.speeds, function (index, speed) { var link, listItem; - link = '' + speed + 'x'; + link = '' + speed + 'x'; - listItem = $('
  • ' + link + '
    • '); + listItem = $('
  • ' + link + '
    • '); if (speed === params.currentSpeed) { listItem.addClass('active'); diff --git a/lms/templates/video.html b/lms/templates/video.html index 2bf21478e4..e7900c4fd7 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -71,7 +71,7 @@

    ${_('Speed')}

    -
      +