Skip to content

E
edx-platform
Commit 1e686f47 by Ari Rizzitano
Options

[video] fix AC-343 

[captions] move role attr from li element to nested span (AC-343)

[video] fix failing tests for video captions (AC-343)

[video] couple more failing tests (AC-343)

[video] properly outline the focused caption (AC-343)
parent 443edadc
Showing with 58 additions and 51 deletions
common/lib/xmodule/xmodule/css/video/display.scss
...@@ -705,7 +705,7 @@ ...@@ -705,7 +705,7 @@
.subtitles-menu { .subtitles-menu {
height: 100%; height: 100%;
margin: 0; margin: 0;
padding: 0; padding: 0 3px;
list-style: none; list-style: none;
li { li {
...@@ -716,6 +716,10 @@ ...@@ -716,6 +716,10 @@
color: #0074b5; // AA compliant color: #0074b5; // AA compliant
line-height: lh(); line-height: lh();
span {
display: block;
}
&.current { &.current {
color: #333; color: #333;
font-weight: 700; font-weight: 700;
......
common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js
...@@ -333,10 +333,10 @@ ...@@ -333,10 +333,10 @@
it('render the transcript', function() { it('render the transcript', function() {
var captionsData = jasmine.stubbedCaption, var captionsData = jasmine.stubbedCaption,
items = $('.subtitles li[data-index]'); $items = $('.subtitles li span[data-index]');
_.each(captionsData.text, function(text, index) { _.each(captionsData.text, function(text, index) {
var item = items.eq(index); var item = $items.eq(index);
expect(parseIntAttribute(item, 'data-index')).toEqual(index); expect(parseIntAttribute(item, 'data-index')).toEqual(index);
expect(parseIntAttribute(item, 'data-start')).toEqual(captionsData.start[index]); expect(parseIntAttribute(item, 'data-start')).toEqual(captionsData.start[index]);
...@@ -362,7 +362,7 @@ ...@@ -362,7 +362,7 @@
$.each(handlerList, function(index, handler) { $.each(handlerList, function(index, handler) {
spyOn(state.videoCaption, handler); spyOn(state.videoCaption, handler);
}); });
$('.subtitles li[data-index]').each( $('.subtitles li span[data-index]').each(
function(index, link) { function(index, link) {
$(link).trigger('mouseover'); $(link).trigger('mouseover');
expect(state.videoCaption.captionMouseOverOut).toHaveBeenCalled(); expect(state.videoCaption.captionMouseOverOut).toHaveBeenCalled();
...@@ -420,10 +420,10 @@ ...@@ -420,10 +420,10 @@
return state.videoCaption.rendered; return state.videoCaption.rendered;
}).then(function() { }).then(function() {
var captionsData = jasmine.stubbedCaption, var captionsData = jasmine.stubbedCaption,
items = $('.subtitles li[data-index]'); $items = $('.subtitles li span[data-index]');
_.each(captionsData.text, function(text, index) { _.each(captionsData.text, function(text, index) {
var item = items.eq(index); var item = $items.eq(index);
expect(parseIntAttribute(item, 'data-index')).toEqual(index); expect(parseIntAttribute(item, 'data-index')).toEqual(index);
expect(parseIntAttribute(item, 'data-start')).toEqual(captionsData.start[index]); expect(parseIntAttribute(item, 'data-start')).toEqual(captionsData.start[index]);
...@@ -533,7 +533,8 @@ ...@@ -533,7 +533,8 @@
describe('when the player is playing', function() { describe('when the player is playing', function() {
beforeEach(function() { beforeEach(function() {
state.videoCaption.playing = true; state.videoCaption.playing = true;
$('.subtitles-menu li[data-index]:first') $('.subtitles-menu span[data-index]:first')
.parent()
.addClass('current'); .addClass('current');
$('.subtitles-menu').trigger(jQuery.Event('mouseout')); $('.subtitles-menu').trigger(jQuery.Event('mouseout'));
}); });
...@@ -832,7 +833,7 @@ ...@@ -832,7 +833,7 @@
captionsData = jasmine.stubbedCaption; captionsData = jasmine.stubbedCaption;
$('.subtitles li[data-index]').each( $('.subtitles li span[data-index]').each(
function(index, item) { function(index, item) {
expect(parseIntAttribute($(item), 'data-index')).toEqual(index); expect(parseIntAttribute($(item), 'data-index')).toEqual(index);
expect(parseIntAttribute($(item), 'data-start')).toEqual(captionsData.start[index]); expect(parseIntAttribute($(item), 'data-start')).toEqual(captionsData.start[index]);
...@@ -913,17 +914,17 @@ ...@@ -913,17 +914,17 @@
describe('when the index is not the same', function() { describe('when the index is not the same', function() {
beforeEach(function() { beforeEach(function() {
state.videoCaption.currentIndex = 1; state.videoCaption.currentIndex = 1;
$('.subtitles li[data-index=5]').addClass('current'); $('.subtitles li span[data-index=5]').addClass('current');
state.videoCaption.updatePlayTime(25.000); state.videoCaption.updatePlayTime(25.000);
}); });
it('deactivate the previous transcript', function() { it('deactivate the previous transcript', function() {
expect($('.subtitles li[data-index=1]')) expect($('.subtitles li span[data-index=1]'))
.not.toHaveClass('current'); .not.toHaveClass('current');
}); });
it('activate new transcript', function() { it('activate new transcript', function() {
expect($('.subtitles li[data-index=5]')) expect($('.subtitles li span[data-index=5]'))
.toHaveClass('current'); .toHaveClass('current');
}); });
...@@ -939,9 +940,9 @@ ...@@ -939,9 +940,9 @@
describe('when the index is the same', function() { describe('when the index is the same', function() {
it('does not change current subtitle', function() { it('does not change current subtitle', function() {
state.videoCaption.currentIndex = 1; state.videoCaption.currentIndex = 1;
$('.subtitles li[data-index=3]').addClass('current'); $('.subtitles li span[data-index=3]').addClass('current');
state.videoCaption.updatePlayTime(15.000); state.videoCaption.updatePlayTime(15.000);
expect($('.subtitles li[data-index=3]')) expect($('.subtitles li span[data-index=3]'))
.toHaveClass('current'); .toHaveClass('current');
}); });
}); });
...@@ -955,7 +956,7 @@ ...@@ -955,7 +956,7 @@
return state.videoCaption.rendered; return state.videoCaption.rendered;
}).then(function() { }).then(function() {
videoControl = state.videoControl; videoControl = state.videoControl;
$('.subtitles li[data-index=1]').addClass('current'); $('.subtitles li span[data-index=1]').addClass('current');
state.videoCaption.onResize(); state.videoCaption.onResize();
}).always(done); }).always(done);
}); });
...@@ -1029,7 +1030,7 @@ ...@@ -1029,7 +1030,7 @@
it('does not scroll the transcript', function() { it('does not scroll the transcript', function() {
runs(function() { runs(function() {
state.videoCaption.frozen = true; state.videoCaption.frozen = true;
$('.subtitles li[data-index=1]').addClass('current'); $('.subtitles li span[data-index=1]').addClass('current');
state.videoCaption.scrollCaption(); state.videoCaption.scrollCaption();
expect($.fn.scrollTo).not.toHaveBeenCalled(); expect($.fn.scrollTo).not.toHaveBeenCalled();
}); });
...@@ -1055,7 +1056,7 @@ ...@@ -1055,7 +1056,7 @@
describe('when there is a current transcript', function() { describe('when there is a current transcript', function() {
it('scroll to current transcript', function() { it('scroll to current transcript', function() {
runs(function() { runs(function() {
$('.subtitles li[data-index=1]').addClass('current'); $('.subtitles li span[data-index=1]').addClass('current');
state.videoCaption.scrollCaption(); state.videoCaption.scrollCaption();
expect($.fn.scrollTo).toHaveBeenCalled(); expect($.fn.scrollTo).toHaveBeenCalled();
}); });
...@@ -1082,7 +1083,7 @@ ...@@ -1082,7 +1083,7 @@
it('trigger seek event with the correct time', function() { it('trigger seek event with the correct time', function() {
runs(function() { runs(function() {
state.videoSpeedControl.currentSpeed = '1.0'; state.videoSpeedControl.currentSpeed = '1.0';
$('.subtitles li[data-start="14910"]').trigger('click'); $('.subtitles li span[data-start="14910"]').trigger('click');
expect(state.videoPlayer.currentTime).toEqual(14.91); expect(state.videoPlayer.currentTime).toEqual(14.91);
}); });
}); });
...@@ -1092,7 +1093,7 @@ ...@@ -1092,7 +1093,7 @@
it('trigger seek event with the correct time', function() { it('trigger seek event with the correct time', function() {
runs(function() { runs(function() {
state.videoSpeedControl.currentSpeed = '0.75'; state.videoSpeedControl.currentSpeed = '0.75';
$('.subtitles li[data-start="14910"]').trigger('click'); $('.subtitles li span[data-start="14910"]').trigger('click');
expect(state.videoPlayer.currentTime).toEqual(14.91); expect(state.videoPlayer.currentTime).toEqual(14.91);
}); });
}); });
...@@ -1104,7 +1105,7 @@ ...@@ -1104,7 +1105,7 @@
runs(function() { runs(function() {
state.videoSpeedControl.currentSpeed = '0.75'; state.videoSpeedControl.currentSpeed = '0.75';
state.currentPlayerMode = 'flash'; state.currentPlayerMode = 'flash';
$('.subtitles li[data-start="14910"]').trigger('click'); $('.subtitles li span[data-start="14910"]').trigger('click');
expect(state.videoPlayer.currentTime).toEqual(15); expect(state.videoPlayer.currentTime).toEqual(15);
}); });
}); });
...@@ -1114,7 +1115,7 @@ ...@@ -1114,7 +1115,7 @@
describe('toggle', function() { describe('toggle', function() {
beforeEach(function() { beforeEach(function() {
state = jasmine.initializePlayer(); state = jasmine.initializePlayer();
$('.subtitles li[data-index=1]').addClass('current'); $('.subtitles li span[data-index=1]').addClass('current');
}); });
describe('when the transcript is visible', function() { describe('when the transcript is visible', function() {
...@@ -1172,13 +1173,13 @@ ...@@ -1172,13 +1173,13 @@
describe('when getting focus through TAB key', function() { describe('when getting focus through TAB key', function() {
beforeEach(function() { beforeEach(function() {
state.videoCaption.isMouseFocus = false; state.videoCaption.isMouseFocus = false;
$('.subtitles li[data-index=0]').trigger( $('.subtitles li span[data-index=0]').trigger(
jQuery.Event('focus') jQuery.Event('focus')
); );
}); });
it('shows an outline around the transcript', function() { it('shows an outline around the transcript', function() {
expect($('.subtitles li[data-index=0]')) expect($('.subtitles span[data-index=0]').parent())
.toHaveClass('focused'); .toHaveClass('focused');
}); });
...@@ -1189,13 +1190,13 @@ ...@@ -1189,13 +1190,13 @@
describe('when loosing focus through TAB key', function() { describe('when loosing focus through TAB key', function() {
beforeEach(function() { beforeEach(function() {
$('.subtitles li[data-index=0]').trigger( $('.subtitles li span[data-index=0]').trigger(
jQuery.Event('blur') jQuery.Event('blur')
); );
}); });
it('does not show an outline around the transcript', function() { it('does not show an outline around the transcript', function() {
expect($('.subtitles li[data-index=0]')) expect($('.subtitles li span[data-index=0]'))
.not.toHaveClass('focused'); .not.toHaveClass('focused');
}); });
...@@ -1210,14 +1211,14 @@ ...@@ -1210,14 +1211,14 @@
function() { function() {
beforeEach(function() { beforeEach(function() {
state.videoCaption.isMouseFocus = false; state.videoCaption.isMouseFocus = false;
$('.subtitles li[data-index=0]') $('.subtitles li span[data-index=0]')
.trigger(jQuery.Event('focus')); .trigger(jQuery.Event('focus'));
$('.subtitles li[data-index=0]') $('.subtitles li span[data-index=0]')
.trigger(jQuery.Event('mousedown')); .trigger(jQuery.Event('mousedown'));
}); });
it('does not show an outline around it', function() { it('does not show an outline around it', function() {
expect($('.subtitles li[data-index=0]')) expect($('.subtitles li span[data-index=0]'))
.not.toHaveClass('focused'); .not.toHaveClass('focused');
}); });
...@@ -1230,28 +1231,28 @@ ...@@ -1230,28 +1231,28 @@
'when a second transcript gets focus through mouse after ' + 'when a second transcript gets focus through mouse after ' +
'first had focus through TAB key', 'first had focus through TAB key',
function() { function() {
var subDataLiIdx__0, subDataLiIdx__1; var $subDataLiIdx0, $subDataLiIdx1;
beforeEach(function() { beforeEach(function() {
subDataLiIdx__0 = $('.subtitles li[data-index=0]'); $subDataLiIdx0 = $('.subtitles li span[data-index=0]');
subDataLiIdx__1 = $('.subtitles li[data-index=1]'); $subDataLiIdx1 = $('.subtitles li span[data-index=1]');
state.videoCaption.isMouseFocus = false; state.videoCaption.isMouseFocus = false;
subDataLiIdx__0.trigger(jQuery.Event('focus')); $subDataLiIdx0.trigger(jQuery.Event('focus'));
subDataLiIdx__0.trigger(jQuery.Event('blur')); $subDataLiIdx0.trigger(jQuery.Event('blur'));
state.videoCaption.isMouseFocus = true; state.videoCaption.isMouseFocus = true;
subDataLiIdx__1.trigger(jQuery.Event('mousedown')); $subDataLiIdx1.trigger(jQuery.Event('mousedown'));
}); });
it('does not show an outline around the first', function() { it('does not show an outline around the first', function() {
expect(subDataLiIdx__0).not.toHaveClass('focused'); expect($subDataLiIdx0).not.toHaveClass('focused');
}); });
it('does not show an outline around the second', function() { it('does not show an outline around the second', function() {
expect(subDataLiIdx__1).not.toHaveClass('focused'); expect($subDataLiIdx1).not.toHaveClass('focused');
}); });
it('has automatic scrolling enabled', function() { it('has automatic scrolling enabled', function() {
......
common/lib/xmodule/xmodule/js/src/video/09_video_caption.js
...@@ -161,7 +161,7 @@ ...@@ -161,7 +161,7 @@
mousewheel: this.onMovement, mousewheel: this.onMovement,
DOMMouseScroll: this.onMovement DOMMouseScroll: this.onMovement
}) })
.on(events, 'li[data-index]', this.onCaptionHandler); .on(events, 'span[data-index]', this.onCaptionHandler);
this.container.on({ this.container.on({
mouseenter: this.onContainerMouseEnter, mouseenter: this.onContainerMouseEnter,
mouseleave: this.onContainerMouseLeave mouseleave: this.onContainerMouseLeave
...@@ -739,16 +739,16 @@ ...@@ -739,16 +739,16 @@
*/ */
buildCaptions: function(container, start, captions) { buildCaptions: function(container, start, captions) {
var process = function(text, index) { var process = function(text, index) {
var liEl = $('<li>', { var $spanEl = $('<span>', {
'role': 'link', 'role': 'link',
'data-index': index, 'data-index': index,
'data-start': start[index], 'data-start': start[index],
'tabindex': 0 'tabindex': 0
}); });
HtmlUtils.setHtml($(liEl), HtmlUtils.HTML(text.toString())); HtmlUtils.setHtml($($spanEl), HtmlUtils.HTML(text.toString()));
return liEl[0]; return $spanEl.wrap('<li>').parent()[0]; // safe-lint: disable=javascript-jquery-insertion
}; };
return AsyncProcess.array(captions, process).done(function(list) { return AsyncProcess.array(captions, process).done(function(list) {
...@@ -912,20 +912,21 @@ ...@@ -912,20 +912,21 @@
*/ */
captionFocus: function(event) { captionFocus: function(event) {
var caption = $(event.target), var caption = $(event.target),
container = caption.parent(),
captionIndex = parseInt(caption.attr('data-index'), 10); captionIndex = parseInt(caption.attr('data-index'), 10);
// If the focus comes from a mouse click, hide the outline, turn on // If the focus comes from a mouse click, hide the outline, turn on
// automatic scrolling and set currentCaptionIndex to point outside of // automatic scrolling and set currentCaptionIndex to point outside of
// caption list (ie -1) to disable mouseenter, mouseleave behavior. // caption list (ie -1) to disable mouseenter, mouseleave behavior.
if (this.isMouseFocus) { if (this.isMouseFocus) {
this.autoScrolling = true; this.autoScrolling = true;
caption.removeClass('focused'); container.removeClass('focused');
this.currentCaptionIndex = -1; this.currentCaptionIndex = -1;
} }
// If the focus comes from tabbing, show the outline and turn off // If the focus comes from tabbing, show the outline and turn off
// automatic scrolling. // automatic scrolling.
else { else {
this.currentCaptionIndex = captionIndex; this.currentCaptionIndex = captionIndex;
caption.addClass('focused'); container.addClass('focused');
// The second and second to last elements turn automatic scrolling // The second and second to last elements turn automatic scrolling
// off again as it may have been enabled in captionBlur. // off again as it may have been enabled in captionBlur.
if ( if (
...@@ -945,9 +946,10 @@ ...@@ -945,9 +946,10 @@
*/ */
captionBlur: function(event) { captionBlur: function(event) {
var caption = $(event.target), var caption = $(event.target),
container = caption.parent(),
captionIndex = parseInt(caption.attr('data-index'), 10); captionIndex = parseInt(caption.attr('data-index'), 10);
caption.removeClass('focused'); container.removeClass('focused');
// If we are on first or last index, we have to turn automatic scroll // If we are on first or last index, we have to turn automatic scroll
// on again when losing focus. There is no way to know in what // on again when losing focus. There is no way to know in what
// direction we are tabbing. So we could be on the first element and // direction we are tabbing. So we could be on the first element and
...@@ -1057,11 +1059,12 @@ ...@@ -1057,11 +1059,12 @@
} }
this.subtitlesEl this.subtitlesEl
.find("li[data-index='" + newIndex + "']") .find("span[data-index='" + newIndex + "']")
.parent()
.addClass('current'); .addClass('current');
this.currentIndex = newIndex; this.currentIndex = newIndex;
this.captionDisplayEl.text(this.subtitlesEl.find("li[data-index='" + newIndex + "']").text()); this.captionDisplayEl.text(this.subtitlesEl.find("span[data-index='" + newIndex + "']").text());
this.scrollCaption(); this.scrollCaption();
} }
} }
......
common/test/acceptance/pages/lms/video/video.py
...@@ -32,8 +32,8 @@ CSS_CLASS_NAMES = { ...@@ -32,8 +32,8 @@ CSS_CLASS_NAMES = {
'captions_closed': '.video.closed', 'captions_closed': '.video.closed',
'captions_rendered': '.video.is-captions-rendered', 'captions_rendered': '.video.is-captions-rendered',
'captions': '.subtitles', 'captions': '.subtitles',
'captions_text': '.subtitles li', 'captions_text': '.subtitles li span',
'captions_text_getter': '.subtitles li[role="link"][data-index="1"]', 'captions_text_getter': '.subtitles li span[role="link"][data-index="1"]',
'closed_captions': '.closed-captions', 'closed_captions': '.closed-captions',
'error_message': '.video .video-player .video-error', 'error_message': '.video .video-player .video-error',
'video_container': '.video', 'video_container': '.video',
......
common/test/acceptance/pages/studio/video/video.py
...@@ -277,7 +277,7 @@ class VideoComponentPage(VideoPage): ...@@ -277,7 +277,7 @@ class VideoComponentPage(VideoPage):
line_number (int): caption line number line_number (int): caption line number
""" """
caption_line_selector = ".subtitles li[data-index='{index}']".format(index=line_number - 1) caption_line_selector = ".subtitles li span[data-index='{index}']".format(index=line_number - 1)
self.q(css=caption_line_selector).results[0].send_keys(Keys.ENTER) self.q(css=caption_line_selector).results[0].send_keys(Keys.ENTER)
def is_caption_line_focused(self, line_number): def is_caption_line_focused(self, line_number):
...@@ -288,10 +288,9 @@ class VideoComponentPage(VideoPage): ...@@ -288,10 +288,9 @@ class VideoComponentPage(VideoPage):
line_number (int): caption line number line_number (int): caption line number
""" """
caption_line_selector = ".subtitles li[data-index='{index}']".format(index=line_number - 1) caption_line_selector = ".subtitles li span[data-index='{index}']".format(index=line_number - 1)
attributes = self.q(css=caption_line_selector).attrs('class') caption_container = self.q(css=caption_line_selector).results[0].find_element_by_xpath('..')
return 'focused' in caption_container.get_attribute('class').split()
return 'focused' in attributes
@property @property
def is_slider_range_visible(self): def is_slider_range_visible(self):
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment