From 578e02b589a89b534e38630b305c29cb5857439a Mon Sep 17 00:00:00 2001 From: jmclaus Date: Tue, 3 Sep 2013 21:01:42 +0200 Subject: [PATCH 1/3] Speed button know behaves like the volume button when tabbing forward or backward. --- .../js/src/video/07_video_volume_control.js | 3 + .../js/src/video/08_video_speed_control.js | 112 +++++++++++------- 2 files changed, 73 insertions(+), 42 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js b/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js index 90154d2079..b378ba7bd0 100644 --- a/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js +++ b/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js @@ -125,6 +125,9 @@ function () { // We store the fact that previous element that lost focus was // the volume clontrol. state.volumeBlur = true; + // The following field is used in video_speed_control to track + // the element that had the focus. + state.previousFocus = 'volume'; }); } 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 91d2ba6fba..687359c183 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 @@ -153,69 +153,97 @@ function () { state.videoSpeedControl.el.removeClass('open'); }); + + // ****************************** + // The tabbing will cycle through the elements in the following + // order: + // 1. Play button + // 2. Speed button + // 3. Fastest speed called firstSpeed + // 4. Intermediary speed called otherSpeed + // 5. Slowest speed called lastSpeed + // 6. Sound button + // This field will keep track of where the focus is coming from. + state.previousFocus = ''; + + // ****************************** // Attach 'focus', and 'blur' events to the speed button 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 comming from the first speed entry, this - // means we are tabbing backwards. In this case we have to - // hide the speed entries which will allow us to change the - // focus further backwards. - if (state.firstSpeedBlur === true) { - state.videoSpeedControl.el.removeClass('open'); - - state.firstSpeedBlur = false; - } - - // If the focus is comming from some other element, show - // the drop down with the speed entries. - else { - state.videoSpeedControl.el.addClass('open'); + // 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, if the speed entries - // dialog is shown (tabbing forwards), then we will set - // focus to the first speed entry. - // - // If the selector does not select anything, then this - // means that the speed entries dialog is closed, and we - // are tabbing backwads. The browser will select the - // previous element to tab to by itself. - state.videoSpeedControl.videoSpeedsEl + // When the focus leaves this element, the speed entries + // dialog will be shown. + + // If we are tabbing forward (previous focus is empty ie + // play button), we open the dialog and set focus on the + // first speed entry. + if (state.previousFocus === '') { + state.videoSpeedControl.el.addClass('open'); + state.videoSpeedControl.videoSpeedsEl .find('a.speed_link:first') .focus(); + } + + // If we are tabbing backwards (previous focus is volume + // button), 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(); + } + + state.previousFocus = ''; }); // ****************************** - // Attach 'focus', and 'blur' events to elements which represent - // individual speed entries. + // 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.last().on('blur', function () { - // If we have reached the last speed entry, and the focus - // changes to the next element, we need to hide the speeds - // control drop-down. - state.videoSpeedControl.el.removeClass('open'); - }); speedLinks.first().on('blur', function () { - // This flag will indicate that the focus to the next - // element that will receive it is comming from the first - // speed entry. - // - // This flag will be used to correctly handle scenario of - // tabbing backwards. - state.firstSpeedBlur = true; + // The previous focus is a speed entry (we are tabbing + // backwards), the dialog will close, set focus on the speed + // button and track the focus on first speed. + if (state.previousFocus === 'otherSpeed') { + state.previousFocus = 'firstSpeed'; + state.videoSpeedControl.el.children('a').focus(); + } }); - speedLinks.on('focus', function () { - // Clear the flag which is only set when we are un-focusing - // (the blur event) from the first speed entry. - state.firstSpeedBlur = false; + + // 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 + // button and track the focus on last speed. + if (state.previousFocus === 'otherSpeed') { + state.previousFocus = 'lastSpeed'; + state.videoSpeedControl.el.children('a').focus(); + } }); + } } From 8e2c5bca7bbf3f35e2ace8a8979c9e3ef120d6fb Mon Sep 17 00:00:00 2001 From: jmclaus Date: Wed, 4 Sep 2013 12:11:23 +0200 Subject: [PATCH 2/3] Added focus tracking to play/pause button. This fixes a bug where the tabbing, in certain condition, would get stuck on speed button. --- common/lib/xmodule/xmodule/js/src/video/04_video_control.js | 5 +++++ .../xmodule/xmodule/js/src/video/07_video_volume_control.js | 2 +- .../xmodule/xmodule/js/src/video/08_video_speed_control.js | 3 +-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/common/lib/xmodule/xmodule/js/src/video/04_video_control.js b/common/lib/xmodule/xmodule/js/src/video/04_video_control.js index 63c4cd5e85..69ebaf18c6 100644 --- a/common/lib/xmodule/xmodule/js/src/video/04_video_control.js +++ b/common/lib/xmodule/xmodule/js/src/video/04_video_control.js @@ -77,6 +77,11 @@ function () { state.el.on('mousemove', state.videoControl.showControls); state.el.on('keydown', state.videoControl.showControls); } + // The state.previousFocus is used in video_speed_control to track + // the element that had the focus before it. + state.videoControl.playPauseEl.on('blur', function () { + state.previousFocus = 'playPause'; + }); } // *************************************************************** diff --git a/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js b/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js index b378ba7bd0..d8398ab530 100644 --- a/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js +++ b/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js @@ -126,7 +126,7 @@ function () { // the volume clontrol. state.volumeBlur = true; // The following field is used in video_speed_control to track - // the element that had the focus. + // the element that had the focus before it. state.previousFocus = 'volume'; }); } 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 687359c183..d9b9c300df 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 @@ -188,7 +188,7 @@ function () { // If we are tabbing forward (previous focus is empty ie // play button), we open the dialog and set focus on the // first speed entry. - if (state.previousFocus === '') { + if (state.previousFocus === 'playPause') { state.videoSpeedControl.el.addClass('open'); state.videoSpeedControl.videoSpeedsEl .find('a.speed_link:first') @@ -205,7 +205,6 @@ function () { .focus(); } - state.previousFocus = ''; }); From e1f75666fae7a31c51de195fd653392eff43e118 Mon Sep 17 00:00:00 2001 From: jmclaus Date: Thu, 5 Sep 2013 22:39:38 +0200 Subject: [PATCH 3/3] Added test to verify that there is a certain order in the controls -- tabbing on the speed control will malfunction if it is not the case. --- .../js/spec/video/video_speed_control_spec.js | 32 ++++++++++++++++++ .../js/src/video/08_video_speed_control.js | 33 +++++++++---------- 2 files changed, 47 insertions(+), 18 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 f8f2b63123..af919e135e 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 @@ -82,6 +82,38 @@ $('.speeds').mouseenter().click(); expect($('.speeds')).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 an other 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 playIndex, speedIndex, firstSpeedEntry, lastSpeedEntry, volumeIndex, foundFirst = false; + $('.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; + } + lastSpeedEntry = index; + } + else if ($(this).parent().hasClass('volume')) { + volumeIndex = index; + } + }); + expect(playIndex+1).toEqual(speedIndex); + expect(speedIndex+1).toEqual(firstSpeedEntry); + expect(lastSpeedEntry+1).toEqual(volumeIndex); + }); }); }); 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 d9b9c300df..6a6587186d 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 @@ -153,22 +153,20 @@ function () { state.videoSpeedControl.el.removeClass('open'); }); - // ****************************** // The tabbing will cycle through the elements in the following // order: - // 1. Play button - // 2. Speed button + // 1. Play control + // 2. Speed control // 3. Fastest speed called firstSpeed // 4. Intermediary speed called otherSpeed // 5. Slowest speed called lastSpeed - // 6. Sound button + // 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 button which + // 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') @@ -185,9 +183,9 @@ function () { // When the focus leaves this element, the speed entries // dialog will be shown. - // If we are tabbing forward (previous focus is empty ie - // play button), we open the dialog and set focus on the - // first speed entry. + // 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 @@ -196,7 +194,7 @@ function () { } // If we are tabbing backwards (previous focus is volume - // button), we open the dialog and set focus on the + // control), we open the dialog and set focus on the // last speed entry. if (state.previousFocus === 'volume') { state.videoSpeedControl.el.addClass('open'); @@ -207,17 +205,16 @@ function () { }); - // ****************************** - // Attach 'blur' event to elements which represent individual - // speed entries and use it to track the origin of the 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 + // The previous focus is a speed entry (we are tabbing // backwards), the dialog will close, set focus on the speed - // button and track the focus on first speed. + // control and track the focus on first speed. if (state.previousFocus === 'otherSpeed') { state.previousFocus = 'firstSpeed'; state.videoSpeedControl.el.children('a').focus(); @@ -234,9 +231,9 @@ function () { }); 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 - // button and track the focus on last speed. + // 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();