Commit 4a921e47 by Valera Rozuvan

Merge pull request #1397 from edx/valera/add_visual_feedback_start_end_time_video

Add visual indicator to the interface for start/end times
parents 0a9db940 1a9214b2
......@@ -5,6 +5,10 @@ These are notable changes in edx-platform. This is a rolling list of changes,
in roughly chronological order, most recent first. Add your entries at or near
the top. Include a label indicating the component affected.
Blades: When start time and end time are specified for a video, a visual range
will be shown on the time slider to highlight the place in the video that will
be played.
Studio: Bug fix for text loss in Course Updates when the text exists
before the first tag.
......@@ -367,10 +371,10 @@ Studio: Improve link re-writing on imports into a different course-id
Studio: course catalog and course outline pages new use course id syntax w/ restful api style
Common:
Common:
separate the non-sql db connection configuration from the modulestore (xblock modeling) configuration.
in split, separate the the db connection and atomic crud ops into a distinct module & class from modulestore
Common: location mapper: % encode periods and dollar signs when used as key in the mapping dict
Common: location mapper: added a bunch of new helper functions for generating old location style info from a CourseLocator
......
......@@ -78,7 +78,7 @@ Feature: Video Component Editor
And I enter a "http://youtu.be/t__eq_exist" source to field number 1
Then I see status message "not found"
And I see button "import"
And I click button "import"
And I click transcript button "import"
Then I see status message "found"
And I see button "upload_new_timed_transcripts"
And I see button "download_to_edit"
......@@ -111,7 +111,7 @@ Feature: Video Component Editor
And I enter a "http://youtu.be/t_neq_exist" source to field number 1
And I see status message "replace"
And I see button "replace"
And I click button "replace"
And I click transcript button "replace"
And I see status message "found"
And I see value "t_neq_exist" in the field "HTML5 Transcript"
......@@ -153,7 +153,7 @@ Feature: Video Component Editor
And I enter a "http://youtu.be/t__eq_exist" source to field number 1
Then I see status message "not found"
And I see button "import"
And I click button "import"
And I click transcript button "import"
Then I see status message "found"
And I enter a "t_not_exist.mp4" source to field number 2
......@@ -205,7 +205,7 @@ Feature: Video Component Editor
And I enter a "http://youtu.be/t__eq_exist" source to field number 1
Then I see status message "not found"
And I see button "import"
And I click button "import"
And I click transcript button "import"
Then I see status message "found"
And I see button "upload_new_timed_transcripts"
......@@ -287,7 +287,7 @@ Feature: Video Component Editor
And I enter a "http://youtu.be/t__eq_exist" source to field number 1
Then I see status message "not found"
And I see button "import"
And I click button "import"
And I click transcript button "import"
Then I see status message "found"
And I see button "upload_new_timed_transcripts"
......@@ -309,7 +309,7 @@ Feature: Video Component Editor
And I enter a "http://youtu.be/t__eq_exist" source to field number 1
Then I see status message "not found"
And I see button "import"
And I click button "import"
And I click transcript button "import"
Then I see status message "found"
And I see button "upload_new_timed_transcripts"
......@@ -364,7 +364,7 @@ Feature: Video Component Editor
And I see choose button "test_transcripts.mp4" number 1
And I see choose button "t_not_exist.webm" number 2
And I click button "choose" number 2
And I click transcript button "choose" number 2
And I see value "test_transcripts|t_not_exist" in the field "HTML5 Transcript"
#21
......@@ -384,13 +384,13 @@ Feature: Video Component Editor
And I enter a "video_name_2.mp4" source to field number 1
Then I see status message "use existing"
And I see button "use_existing"
And I click button "use_existing"
And I click transcript button "use_existing"
And I see value "video_name_2" in the field "HTML5 Transcript"
And I enter a "video_name_3.mp4" source to field number 1
Then I see status message "use existing"
And I see button "use_existing"
And I click button "use_existing"
And I click transcript button "use_existing"
And I see value "video_name_3" in the field "HTML5 Transcript"
#22
......@@ -410,7 +410,7 @@ Feature: Video Component Editor
And I enter a "video_name_2.mp4" source to field number 1
Then I see status message "use existing"
And I see button "use_existing"
And I click button "use_existing"
And I click transcript button "use_existing"
And I see value "video_name_2" in the field "HTML5 Transcript"
And I enter a "video_name_3.mp4" source to field number 1
......@@ -420,7 +420,7 @@ Feature: Video Component Editor
And I enter a "video_name_4.mp4" source to field number 1
Then I see status message "use existing"
And I see button "use_existing"
And I click button "use_existing"
And I click transcript button "use_existing"
And I see value "video_name_4" in the field "HTML5 Transcript"
#23
......@@ -443,7 +443,7 @@ Feature: Video Component Editor
And I enter a "video_name_3.webm" source to field number 2
Then I see status message "use existing"
And I see button "use_existing"
And I click button "use_existing"
And I click transcript button "use_existing"
And I see value "video_name_2|video_name_3" in the field "HTML5 Transcript"
#24 Uploading subtitles with different file name than file
......
......@@ -38,7 +38,7 @@ SELECTORS = {
}
# button type , button css selector, button message
BUTTONS = {
TRANSCRIPTS_BUTTONS = {
'import': ('.setting-import', 'Import from YouTube'),
'download_to_edit': ('.setting-download', 'Download to Edit'),
'disabled_download_to_edit': ('.setting-download.is-disabled', 'Download to Edit'),
......@@ -127,9 +127,9 @@ def i_see_button(_step, not_see, button_type):
button = button_type.strip()
if not_see.strip():
assert world.is_css_not_present(BUTTONS[button][0])
assert world.is_css_not_present(TRANSCRIPTS_BUTTONS[button][0])
else:
assert world.css_has_text(BUTTONS[button][0], BUTTONS[button][1])
assert world.css_has_text(TRANSCRIPTS_BUTTONS[button][0], TRANSCRIPTS_BUTTONS[button][1])
@step('I (.*)see (.*)button "([^"]*)" number (\d+)$')
......@@ -142,21 +142,21 @@ def i_see_button_with_custom_text(_step, not_see, button_type, custom_text, inde
index = int(index.strip()) - 1
if not_see.strip():
assert world.is_css_not_present(BUTTONS[button][0])
assert world.is_css_not_present(TRANSCRIPTS_BUTTONS[button][0])
else:
assert world.css_has_text(BUTTONS[button][0], BUTTONS[button][1].format(custom_text), index)
assert world.css_has_text(TRANSCRIPTS_BUTTONS[button][0], TRANSCRIPTS_BUTTONS[button][1].format(custom_text), index)
@step('I click button "([^"]*)"$')
def click_button(_step, button_type):
@step('I click transcript button "([^"]*)"$')
def click_button_transcripts_variant(_step, button_type):
world.wait(DELAY)
world.wait_for_ajax_complete()
button = button_type.strip()
world.css_click(BUTTONS[button][0])
world.css_click(TRANSCRIPTS_BUTTONS[button][0])
@step('I click button "([^"]*)" number (\d+)$')
@step('I click transcript button "([^"]*)" number (\d+)$')
def click_button_index(_step, button_type, index):
world.wait(DELAY)
world.wait_for_ajax_complete()
......@@ -164,7 +164,7 @@ def click_button_index(_step, button_type, index):
button = button_type.strip()
index = int(index.strip()) - 1
world.css_click(BUTTONS[button][0], index)
world.css_click(TRANSCRIPTS_BUTTONS[button][0], index)
@step('I remove "([^"]+)" transcripts id from store')
......
......@@ -77,3 +77,20 @@ Feature: CMS.Video Component
Then I focus on caption line with data-index 0
Then I press "enter" button on caption line with data-index 0
And I see caption line with data-index 0 has class "focused"
# 11
Scenario: When start end end times are specified, a range on slider is shown
Given I have created a Video component
And Make sure captions are closed
And I edit the component
And I open tab "Advanced"
And I set value "12" to the field "Start Time"
And I set value "24" to the field "End Time"
And I save changes
And I click video button "Play"
# The below line is a bit flaky. Numbers 73 and 73 were determined rather
# accidentally. They might change in the future as Video player gets CSS
# updates. If this test starts failing, 99.9% cause of failure is the line
# below.
Then I see a range on slider with styles "left" set to 73 px and "width" set to 73 px
......@@ -5,11 +5,15 @@ from xmodule.modulestore import Location
from contentstore.utils import get_modulestore
from selenium.webdriver.common.keys import Keys
BUTTONS = {
VIDEO_BUTTONS = {
'CC': '.hide-subtitles',
'volume': '.volume',
'Play': '.video_control.play',
}
# We should wait 300 ms for event handler invocation + 200ms for safety.
DELAY = 0.5
@step('I have created a Video component$')
def i_created_a_video_component(step):
......@@ -131,7 +135,7 @@ def set_captions_visibility_state(_step, captions_state):
@step('I hover over button "([^"]*)"$')
def hover_over_button(_step, button):
world.css_find(BUTTONS[button.strip()]).mouse_over()
world.css_find(VIDEO_BUTTONS[button.strip()]).mouse_over()
@step('Captions (?:are|become) "([^"]*)"$')
......@@ -173,3 +177,24 @@ def caption_line_has_class(_step, index, className):
SELECTOR = ".subtitles > li[data-index='{index}']".format(index=int(index.strip()))
world.css_has_class(SELECTOR, className.strip())
@step('I see a range on slider with styles "left" set to (.+) px and "width" set to (.+) px$')
def see_a_range_slider_with_proper_range(_step, left, width):
left = int(left.strip())
width = int(width.strip())
world.wait_for_visible(".slider-range")
world.wait(4)
slider_range = world.browser.driver.find_element_by_css_selector(".slider-range")
assert int(round(float(slider_range.value_of_css_property("left")[:-2]))) == left
assert int(round(float(slider_range.value_of_css_property("width")[:-2]))) == width
@step('I click video button "([^"]*)"$')
def click_button_video(_step, button_type):
world.wait(DELAY)
world.wait_for_ajax_complete()
button = button_type.strip()
world.css_click(VIDEO_BUTTONS[button])
......@@ -139,6 +139,24 @@ div.video {
box-shadow: inset 0 1px 0 #999;
}
div.ui-corner-all.slider-range {
background-color: #1e91d3;
/* We add opacity so that we can discern the amount of video that has
* been played. The progress will advance as a gray-shaded area. When
* it will overlap with the range, you will see a different shade of
* the range for part that has been played, and for part that is
* still to be played.
*
* For CSS opacity, different browsers are handled differently.
*/
-ms-filter: "progid:DXImageTransform.Microsoft.Alpha(Opacity=30)";
filter: alpha(opacity=30);
-moz-opacity: 0.3;
-khtml-opacity: 0.3;
opacity: 0.3;
}
a.ui-slider-handle {
background: $pink url(../images/slider-handle.png) center center no-repeat;
background-size: 50%;
......
......@@ -184,6 +184,62 @@
});
});
describe('checking start and end times', function () {
var miniTestSuite = [
{
itDescription: 'both times are proper',
data: {start: 12, end: 24},
expectData: {start: 12, end: 24}
},
{
itDescription: 'start time is invalid',
data: {start: '', end: 24},
expectData: {start: 0, end: 24}
},
{
itDescription: 'end time is invalid',
data: {start: 12, end: ''},
expectData: {start: 12, end: null}
},
{
itDescription: 'start time is less than 0',
data: {start: -12, end: 24},
expectData: {start: 0, end: 24}
},
{
itDescription: 'start time is greater than end time',
data: {start: 42, end: 24},
expectData: {start: 42, end: null}
},
];
beforeEach(function () {
loadFixtures('video.html');
});
$.each(miniTestSuite, function (index, test) {
itFabrique(test.itDescription, test.data, test.expectData);
});
return;
function itFabrique(itDescription, data, expectData) {
it(itDescription, function () {
$('#example').find('.video')
.data({
'start': data.start,
'end': data.end
});
state = new Video('#example');
expect(state.config.start).toBe(expectData.start);
expect(state.config.end).toBe(expectData.end);
});
}
});
describe('multiple YT on page', function () {
var state1, state2, state3;
......
......@@ -114,7 +114,9 @@
showinfo: 0,
enablejsapi: 1,
modestbranding: 1,
html5: 1
html5: 1,
start: 0,
end: null
},
videoId: 'cogebirgzzM',
events: {
......
......@@ -75,6 +75,8 @@ function (VideoPlayer) {
state.parseYoutubeStreams = _.bind(parseYoutubeStreams, state);
state.parseVideoSources = _.bind(parseVideoSources, state);
state.getVideoMetadata = _.bind(getVideoMetadata, state);
state.checkStartEndTimes = _.bind(checkStartEndTimes, state);
}
// function _renderElements(state)
......@@ -277,6 +279,10 @@ function (VideoPlayer) {
availableQualities: ['hd720', 'hd1080', 'highres']
};
// Make sure that start end end times are valid. If not, they will be
// set to `null` and will not be used later on.
this.checkStartEndTimes();
// Check if the YT test timeout has been set. If not, or it is in
// improper format, then set to default value.
tempYtTestTimeout = parseInt(data['ytTestTimeout'], 10);
......@@ -360,6 +366,30 @@ function (VideoPlayer) {
}
}
/*
* function checkStartEndTimes()
*
* Validate config.start and config.end times.
*
* We can check at this time if the times are proper integers, and if they
* make general sense. I.e. if start time is => 0 and <= end time.
*
* An invalid start time will be reset to 0. An invalid end time will be
* set to `null`. It the task for the appropriate player API to figure out
* if start time and/or end time are greater than the length of the video.
*/
function checkStartEndTimes() {
this.config.start = parseInt(this.config.start, 10);
if ((!isFinite(this.config.start)) || (this.config.start < 0)) {
this.config.start = 0;
}
this.config.end = parseInt(this.config.end, 10);
if ((!isFinite(this.config.end)) || (this.config.end < this.config.start)) {
this.config.end = null;
}
}
// function parseYoutubeStreams(state, youtubeStreams)
//
// Take a string in the form:
......
......@@ -191,19 +191,8 @@ function () {
}
// Determine the starting and ending time for the video.
this.start = 0;
this.end = null;
if (config.playerVars) {
this.start = parseFloat(config.playerVars.start);
if ((!isFinite(this.start)) || (this.start < 0)) {
this.start = 0;
}
this.end = parseFloat(config.playerVars.end);
if ((!isFinite(this.end)) || (this.end < this.start)) {
this.end = null;
}
}
this.start = config.playerVars.start;
this.end = config.playerVars.end;
// Create HTML markup for the <video> element, populating it with sources from previous step.
// Because of problems with creating video element via jquery
......@@ -217,7 +206,6 @@ function () {
this.videoEl = $(this.video);
this.playerState = HTML5Video.PlayerState.UNSTARTED;
// this.callStateChangeCallback();
// Attach a 'click' event on the <video> element. It will cause the video to pause/play.
this.videoEl.on('click', function (event) {
......
......@@ -34,9 +34,7 @@ function (HTML5Video, Resizer) {
state.videoPlayer.onPause = _.bind(onPause, state);
state.videoPlayer.onPlay = _.bind(onPlay, state);
state.videoPlayer.onUnstarted = _.bind(
onUnstarted, state
);
state.videoPlayer.onUnstarted = _.bind(onUnstarted, state);
state.videoPlayer.handlePlaybackQualityChange = _.bind(
handlePlaybackQualityChange, state
......@@ -86,13 +84,8 @@ function (HTML5Video, Resizer) {
state.videoPlayer.playerVars.html5 = 1;
}
if (state.config.start) {
state.videoPlayer.playerVars.start = state.config.start;
state.videoPlayer.playerVars.wmode = 'window';
}
if (state.config.end) {
state.videoPlayer.playerVars.end = state.config.end;
}
state.videoPlayer.playerVars.start = state.config.start;
state.videoPlayer.playerVars.end = state.config.end;
// There is a bug which prevents YouTube API to correctly set the speed
// to 1.0 from another speed in Firefox when in HTML5 mode. There is a
......@@ -479,6 +472,13 @@ function (HTML5Video, Resizer) {
);
this.trigger(
'videoProgressSlider.updateStartEndTimeRegion',
{
duration: duration
}
);
this.trigger(
'videoControl.updateVcrVidTime',
{
time: time,
......
......@@ -28,21 +28,29 @@ function () {
// function _makeFunctionsPublic(state)
//
// Functions which will be accessible via 'state' object. When called, these functions will
// get the 'state' object as a context.
// Functions which will be accessible via 'state' object. When called,
// these functions will get the 'state' object as a context.
function _makeFunctionsPublic(state) {
state.videoProgressSlider.onSlide = _.bind(onSlide, state);
state.videoProgressSlider.onStop = _.bind(onStop, state);
state.videoProgressSlider.updatePlayTime = _.bind(updatePlayTime, state);
state.videoProgressSlider.updatePlayTime = _.bind(
updatePlayTime, state
);
//Added for tests -- JM
state.videoProgressSlider.buildSlider = _.bind(buildSlider, state);
state.videoProgressSlider.updateStartEndTimeRegion = _.bind(
updateStartEndTimeRegion, state
);
}
// function _renderElements(state)
//
// Create any necessary DOM elements, attach them, and set their initial configuration. Also
// make the created DOM elements available via the 'state' object. Much easier to work this
// way - you don't have to do repeated jQuery element selects.
// Create any necessary DOM elements, attach them, and set their
// initial configuration. Also make the created DOM elements available
// via the 'state' object. Much easier to work this way - you don't
// have to do repeated jQuery element selects.
function _renderElements(state) {
if (!onTouchBasedDevice()) {
state.videoProgressSlider.el = state.videoControl.sliderEl;
......@@ -53,8 +61,9 @@ function () {
}
function _buildHandle(state) {
state.videoProgressSlider.handle = state.videoProgressSlider.el.find('.ui-slider-handle');
state.videoProgressSlider.handle = state.videoProgressSlider.el
.find('.ui-slider-handle');
// ARIA
// We just want the knob to be selectable with keyboard
state.videoProgressSlider.el.attr('tabindex', -1);
......@@ -64,28 +73,84 @@ function () {
'role': 'slider',
'title': 'video position',
'aria-disabled': false,
'aria-valuetext': getTimeDescription(state.videoProgressSlider.slider.slider('option', 'value'))
'aria-valuetext': getTimeDescription(state.videoProgressSlider
.slider.slider('option', 'value'))
});
}
// ***************************************************************
// Public functions start here.
// These are available via the 'state' object. Their context ('this' keyword) is the 'state' object.
// The magic private function that makes them available and sets up their context is makeFunctionsPublic().
// These are available via the 'state' object. Their context ('this'
// keyword) is the 'state' object. The magic private function that makes
// them available and sets up their context is makeFunctionsPublic().
// ***************************************************************
function buildSlider(state) {
state.videoProgressSlider.slider = state.videoProgressSlider.el.slider({
range: 'min',
slide: state.videoProgressSlider.onSlide,
stop: state.videoProgressSlider.onStop
});
state.videoProgressSlider.slider = state.videoProgressSlider.el
.slider({
range: 'min',
slide: state.videoProgressSlider.onSlide,
stop: state.videoProgressSlider.onStop
});
state.videoProgressSlider.sliderProgress = state.videoProgressSlider
.slider
.find('.ui-slider-range.ui-widget-header.ui-slider-range-min');
}
function updateStartEndTimeRegion(params) {
var left, width, start, end;
// We must have a duration in order to determine the area of range.
// It also must be non-zero.
if (!params.duration) {
return;
}
// If the range spans the entire length of video, we don't do anything.
if (!this.config.start && !this.config.end) {
return;
}
start = this.config.start;
// If end is set to null, then we set it to the end of the video. We
// know that start is not a the beginning, therefore we must build a
// range.
end = this.config.end || params.duration;
left = (100 * (start / params.duration)).toFixed(1);
width = (100 * ((end - start) / params.duration)).toFixed(1);
if (!this.videoProgressSlider.sliderRange) {
this.videoProgressSlider.sliderRange = $('<div />', {
class: 'ui-slider-range ' +
'ui-widget-header ' +
'ui-corner-all ' +
'slider-range'
}).css({
left: left + '%',
width: width + '%'
});
this.videoProgressSlider.sliderProgress
.after(this.videoProgressSlider.sliderRange);
} else {
this.videoProgressSlider.sliderRange
.css({
left: left + '%',
width: width + '%'
});
}
}
function onSlide(event, ui) {
this.videoProgressSlider.frozen = true;
this.trigger('videoPlayer.onSlideSeek', {'type': 'onSlideSeek', 'time': ui.value});
this.trigger(
'videoPlayer.onSlideSeek',
{'type': 'onSlideSeek', 'time': ui.value}
);
// ARIA
this.videoProgressSlider.handle.attr(
......@@ -98,7 +163,10 @@ function () {
this.videoProgressSlider.frozen = true;
this.trigger('videoPlayer.onSlideSeek', {'type': 'onSlideSeek', 'time': ui.value});
this.trigger(
'videoPlayer.onSlideSeek',
{'type': 'onSlideSeek', 'time': ui.value}
);
// ARIA
this.videoProgressSlider.handle.attr(
......@@ -110,23 +178,27 @@ function () {
}, 200);
}
//Changed for tests -- JM: Check if it is the cause of Chrome Bug Valera noticed
// Changed for tests -- JM: Check if it is the cause of Chrome Bug Valera
// noticed
function updatePlayTime(params) {
if ((this.videoProgressSlider.slider) && (!this.videoProgressSlider.frozen)) {
/*this.videoProgressSlider.slider
if (
(this.videoProgressSlider.slider) &&
(!this.videoProgressSlider.frozen)
) {
this.videoProgressSlider.slider
.slider('option', 'max', params.duration)
.slider('value', params.time);*/
this.videoProgressSlider.slider.slider('option', 'max', params.duration);
this.videoProgressSlider.slider.slider('option', 'value', params.time);
.slider('option', 'value', params.time);
}
}
// Returns a string describing the current time of video in hh:mm:ss format.
// Returns a string describing the current time of video in hh:mm:ss
// format.
function getTimeDescription(time) {
var seconds = Math.floor(time),
minutes = Math.floor(seconds / 60),
hours = Math.floor(minutes / 60),
hrStr, minStr, secStr;
seconds = seconds % 60;
minutes = minutes % 60;
......@@ -136,30 +208,36 @@ function () {
if (hours) {
hrStr += (hours < 2 ? ' hour ' : ' hours ');
if (minutes) {
if (minutes) {
minStr += (minutes < 2 ? ' minute ' : ' minutes ');
} else {
minStr += ' 0 minutes ';
}
if (seconds) {
if (seconds) {
secStr += (seconds < 2 ? ' second ' : ' seconds ');
} else {
secStr += ' 0 seconds ';
}
}
return hrStr + minStr + secStr;
} else if (minutes) {
minStr += (minutes < 2 ? ' minute ' : ' minutes ');
if (seconds) {
if (seconds) {
secStr += (seconds < 2 ? ' second ' : ' seconds ');
} else {
secStr += ' 0 seconds ';
}
return minStr + secStr;
} else if (seconds) {
secStr += (seconds < 2 ? ' second ' : ' seconds ');
return secStr;
}
return '0 seconds';
}
......
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