From 7c59947ac08d54c446f5a7b9081eb76b406f4e41 Mon Sep 17 00:00:00 2001 From: Vasyl Nakvasiuk Date: Thu, 18 Apr 2013 17:11:08 +0300 Subject: [PATCH] refactoring JS -> HTML second part of refactoring: JS -> Python HTML Updated JS code to reflect the change that some HTML generation was moved to python side. Fixed fullscreen bug - now controls are shown properly below the video. Made the transparency a bit more faded for captions in normal mode. Fixed bug with captions. Now the attribute show_captions correctly removes them when set to true. Work in progress. --- .../xmodule/css/videoalpha/display.scss | 12 +++++- .../js/src/videoalpha/display/html5_video.js | 21 +++++++--- .../js/src/videoalpha/display/initialize.js | 40 +++++++++++------- .../src/videoalpha/display/video_caption.js | 42 +++++++++++++------ .../src/videoalpha/display/video_control.js | 28 +++++-------- .../js/src/videoalpha/display/video_player.js | 2 + .../display/video_progress_slider.js | 6 +-- .../display/video_quality_control.js | 8 ++-- .../videoalpha/display/video_speed_control.js | 21 ++++------ .../display/video_volume_control.js | 10 +---- .../xmodule/xmodule/js/src/videoalpha/main.js | 7 +++- lms/envs/common.py | 2 +- lms/templates/videoalpha.html | 36 +++++++++++++++- 13 files changed, 152 insertions(+), 83 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/videoalpha/display.scss b/common/lib/xmodule/xmodule/css/videoalpha/display.scss index c677584484..d115c0743c 100644 --- a/common/lib/xmodule/xmodule/css/videoalpha/display.scss +++ b/common/lib/xmodule/xmodule/css/videoalpha/display.scss @@ -519,7 +519,7 @@ div.videoalpha { } ol.subtitles.html5 { - background-color: rgba(243, 243, 243, 0.5); + background-color: rgba(243, 243, 243, 0.8); height: 380px; position: absolute; right: 0; @@ -550,12 +550,22 @@ div.videoalpha { } } + article.video-wrapper div.video-player-pre, article.video-wrapper div.video-player-post { + height: 0px; + } + + article.video-wrapper { + position: static; + } + div.tc-wrapper { @include clearfix; display: table; width: 100%; height: 100%; + position: static; + article.video-wrapper { width: 100%; display: table-cell; diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/html5_video.js b/common/lib/xmodule/xmodule/js/src/videoalpha/display/html5_video.js index 91f36556b7..5372108aba 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/html5_video.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/html5_video.js @@ -17,9 +17,7 @@ define( 'videoalpha/display/html5_video.js', [], function () { - var HTML5Video; - - HTML5Video = {}; + var HTML5Video = {}; HTML5Video.Player = (function () { Player.prototype.callStateChangeCallback = function () { @@ -129,9 +127,21 @@ function () { // If el is string, we assume it is an ID of a DOM element. Get the element, and check that the ID // really belongs to an element. If we didn't get a DOM element, return. At this stage, nothing will // break because other parts of the video player are waiting for 'onReady' callback to be called. + + // REFACTOR: Use .length === 0 + + this.el = $(el); + // REFACTOR: Simplify chck. + if (this.el.length === 0) { + return; + } + + + + if (typeof el === 'string') { this.el = $(el); - + // REFACTOR: Simplify chck. if (this.el.length === 0) { return; } @@ -140,7 +150,7 @@ function () { } else { return; } - + // A simple test to see that the 'config' is a normal object. if ($.isPlainObject(config)) { this.config = config; @@ -294,6 +304,7 @@ function () { } }()); + // REFACTOR: Doc. HTML5Video.PlayerState = { 'UNSTARTED': -1, 'ENDED': 0, diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/initialize.js b/common/lib/xmodule/xmodule/js/src/videoalpha/display/initialize.js index 25a44aee50..049746b47e 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/initialize.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/initialize.js @@ -64,20 +64,20 @@ function (VideoPlayer) { // We store all settings passed to us by the server in one place. These are "read only", so don't // modify them. All variable content lives in 'state' object. state.config = { - 'element': element, + element: element, - 'start': state.el.data('start'), - 'end': state.el.data('end'), + start: state.el.data('start'), + end: state.el.data('end'), - 'caption_data_dir': state.el.data('caption-data-dir'), - 'caption_asset_path': state.el.data('caption-asset-path'), - 'show_captions': (state.el.data('show-captions').toString().toLowerCase === 'true'), - 'youtubeStreams': state.el.data('streams'), + caption_data_dir: state.el.data('caption-data-dir'), + caption_asset_path: state.el.data('caption-asset-path'), + show_captions: (state.el.data('show-captions').toString().toLowerCase() === 'true'), + youtubeStreams: state.el.data('streams'), - 'sub': state.el.data('sub'), - 'mp4Source': state.el.data('mp4-source'), - 'webmSource': state.el.data('webm-source'), - 'oggSource': state.el.data('ogg-source') + sub: state.el.data('sub'), + mp4Source: state.el.data('mp4-source'), + webmSource: state.el.data('webm-source'), + oggSource: state.el.data('ogg-source') }; // Try to parse YouTube stream ID's. If @@ -101,7 +101,7 @@ function (VideoPlayer) { } ); - if (!state.config.sub.length) { + if (!state.config.sub || !state.config.sub.length) { state.config.sub = ''; state.config.show_captions = false; } @@ -149,14 +149,14 @@ function (VideoPlayer) { // in a browser that doesn't fully support HTML5. When we have this setting in cookies, we can select // the proper mode from the start (not having to change mode later on). (function (currentPlayerMode) { - if ((currentPlayerMode !== 'html5') && (currentPlayerMode !== 'flash')) { + if ((currentPlayerMode === 'html5') || (currentPlayerMode === 'flash')) { + state.currentPlayerMode = currentPlayerMode; + } else { $.cookie('current_player_mode', 'html5', { 'expires': 3650, 'path': '/' }); state.currentPlayerMode = 'html5'; - } else { - state.currentPlayerMode = currentPlayerMode; } }($.cookie('current_player_mode'))); @@ -222,7 +222,7 @@ function (VideoPlayer) { state.html5Sources = { 'mp4': null, 'webm': null, 'ogg': null }; $.each(sources, function (name, source) { - if (source.length) { + if (source && source.length) { state.html5Sources[name] = source; } }); @@ -255,6 +255,14 @@ function (VideoPlayer) { } function checkForNativeFunctions() { + // REFACTOR: + // 1.) IE8 doc. + // 2.) Move to separate file. + // 3.) Write about a generic soluction system wide. + + // + // IE browser supports Function.bind() only starting with version 8. + // // The bind function is a recent addition to ECMA-262, 5th edition; as such it may not be present in all // browsers. You can partially work around this by inserting the following code at the beginning of your // scripts, allowing use of much of the functionality of bind() in implementations that do not natively support diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_caption.js b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_caption.js index 8c8511e7c1..095dc784a0 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_caption.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_caption.js @@ -53,10 +53,8 @@ function () { function renderElements(state) { state.videoCaption.loaded = false; - state.videoCaption.subtitlesEl = $('
    '); - state.videoCaption.hideSubtitlesEl = $( - 'Captions' - ); + state.videoCaption.subtitlesEl = state.el.find('ol.subtitles'); + state.videoCaption.hideSubtitlesEl = state.el.find('a.hide-subtitles'); state.el.find('.video-wrapper').after(state.videoCaption.subtitlesEl); state.el.find('.video-controls .secondary-controls').append(state.videoCaption.hideSubtitlesEl); @@ -67,6 +65,7 @@ function () { fetchCaption(state); + // REFACTOR. Const. if (state.videoType === 'html5') { state.videoCaption.fadeOutTimeout = 1400; @@ -81,6 +80,8 @@ function () { function bindHandlers(state) { $(window).bind('resize', state.videoCaption.resize); state.videoCaption.hideSubtitlesEl.click(state.videoCaption.toggle); + + // REFACTOR: Use .on() state.videoCaption.subtitlesEl.mouseenter( state.videoCaption.onMouseEnter ).mouseleave( @@ -136,6 +137,8 @@ function () { this.captionsShowLock = true; + // REFACTOR. + if (this.captionState === 'invisible') { this.videoCaption.subtitlesEl.show(); this.captionState = 'visible'; @@ -168,6 +171,7 @@ function () { _this = this; + // REFACTOR. this.videoCaption.subtitlesEl.fadeOut(1000, function () { _this.captionState = 'invisible'; }); @@ -178,6 +182,7 @@ function () { 'maxHeight': this.videoCaption.captionHeight() }); + // REFACTOR: Chain. this.videoCaption.subtitlesEl.find('.spacing:first').height(this.videoCaption.topSpacingHeight()); this.videoCaption.subtitlesEl.find('.spacing:last').height(this.videoCaption.bottomSpacingHeight()); @@ -189,6 +194,7 @@ function () { clearTimeout(this.videoCaption.frozen); } + // REFACTOR. this.videoCaption.frozen = setTimeout(this.videoCaption.onMouseLeave, 10000); } @@ -209,23 +215,25 @@ function () { } function renderCaption() { - var container, _this; - - _this = this; + var container, _this = this; container = $('
      '); $.each(this.videoCaption.captions, function(index, text) { + // REFACTOR: Use .data() container.append($('
    1. ').html(text).attr({ 'data-index': index, 'data-start': _this.videoCaption.start[index] })); }); + // REFACTOR: Chain. this.videoCaption.subtitlesEl.html(container.html()); this.videoCaption.subtitlesEl.find('li[data-index]').on('click', this.videoCaption.seekPlayer); - this.videoCaption.subtitlesEl.prepend( + this.videoCaption.subtitlesEl + .prepend( $('
    2. ').height(this.videoCaption.topSpacingHeight()) - ).append( + ) + .append( $('
    3. ').height(this.videoCaption.bottomSpacingHeight()) ); @@ -233,10 +241,17 @@ function () { } function scrollCaption() { - if (!this.videoCaption.frozen && this.videoCaption.subtitlesEl.find('.current:first').length) { - this.videoCaption.subtitlesEl.scrollTo(this.videoCaption.subtitlesEl.find('.current:first'), { - 'offset': -this.videoCaption.calculateOffset(this.videoCaption.subtitlesEl.find('.current:first')) - }); + // REFACTOR: Cache current:first + if ( + !this.videoCaption.frozen && + this.videoCaption.subtitlesEl.find('.current:first').length + ) { + this.videoCaption.subtitlesEl.scrollTo( + this.videoCaption.subtitlesEl.find('.current:first'), + { + 'offset': -this.videoCaption.calculateOffset(this.videoCaption.subtitlesEl.find('.current:first')) + } + ); } } @@ -352,6 +367,7 @@ function () { } function captionHeight() { + // REFACTOR: Use property instead of class. if (this.el.hasClass('fullscreen')) { return $(window).height() - this.el.find('.video-controls').height(); } else { diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_control.js b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_control.js index e8c2a83c3f..cc2a85a803 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_control.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_control.js @@ -40,23 +40,10 @@ function () { // 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) { - var el, qTipConfig; - // REFACTOR move templates and css to one file- to python part - el = $( - '
      ' + - '
      ' + - '
        ' + - '
        • ' + - '
      • 0:00 / 0:00
        • ' + - '
      ' + - '
      ' + - 'Fill Browser' + - '
      ' + - '
      ' - ); + var qTipConfig; state.videoControl.el = state.el.find('.video-controls'); - state.videoControl.el.append(el); + // state.videoControl.el.append(el); state.videoControl.sliderEl = state.videoControl.el.find('.slider'); state.videoControl.playPauseEl = state.videoControl.el.find('.video_control'); @@ -83,6 +70,8 @@ function () { } if (state.videoType === 'html5') { + // REFACTOR: constants move to initialize() + state.videoControl.fadeOutTimeout = 1400; state.videoControl.el.addClass('html5'); @@ -118,6 +107,9 @@ function () { this.controlShowLock = true; // Refactor: separate UI state in object. No code duplication. + // REFACTOR: + // 1.) Chain jQuery calls. + // 2.) Drop out common code. if (this.controlState === 'invisible') { this.videoControl.el.show(); this.controlState = 'visible'; @@ -156,6 +148,7 @@ function () { } function play() { + // REFACTOR: this.videoControl.playPauseState should be bool. this.videoControl.playPauseEl.removeClass('play').addClass('pause').attr('title', 'Pause'); this.videoControl.playPauseState = 'playing'; } @@ -181,17 +174,18 @@ function () { if (this.videoControl.fullScreenState) { this.videoControl.fullScreenState = false; this.el.removeClass('fullscreen'); - this.videoControl.fullScreenEl.attr('title', 'Fill browser'); + this.videoControl.fullScreenEl.attr('title', 'Fullscreen'); } else { this.videoControl.fullScreenState = true; this.el.addClass('fullscreen'); - this.videoControl.fullScreenEl.attr('title', 'Exit fill browser'); + this.videoControl.fullScreenEl.attr('title', 'Exit fullscreen'); } this.trigger(['videoCaption', 'resize'], null); } function exitFullScreen(event) { + // REFACTOR: Add variable instead of class. if ((this.el.hasClass('fullscreen')) && (event.keyCode === 27)) { this.videoControl.toggleFullScreen(event); } diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.js b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.js index daf0e8728b..f14769e0b4 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_player.js @@ -249,6 +249,8 @@ function (HTML5Video) { function onReady() { var availablePlaybackRates, baseSpeedSubs, _this; + + // REFACTOR: Check if logic. availablePlaybackRates = this.videoPlayer.player.getAvailablePlaybackRates(); if ((this.currentPlayerMode === 'html5') && (this.videoType === 'youtube')) { diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.js b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.js index d2d9af953b..612beb9abf 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_progress_slider.js @@ -97,7 +97,6 @@ function () { function onSlide(event, ui) { this.videoProgressSlider.frozen = true; this.videoProgressSlider.updateTooltip(ui.value); - this.trigger(['videoPlayer', 'onSeek'], ui.value); } @@ -106,9 +105,7 @@ function () { } function onStop(event, ui) { - var _this; - - _this = this; + var _this = this; this.videoProgressSlider.frozen = true; @@ -125,6 +122,7 @@ function () { function updatePlayTime(params) { if ((this.videoProgressSlider.slider) && (!this.videoProgressSlider.frozen)) { + // REFACTOR: Check if you can chain. this.videoProgressSlider.slider.slider('option', 'max', params.duration); this.videoProgressSlider.slider.slider('value', params.time); } diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_quality_control.js b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_quality_control.js index 370e33ab0f..1d559e45cd 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_quality_control.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_quality_control.js @@ -39,14 +39,12 @@ function () { // 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) { - state.videoQualityControl.el = $( - 'HD' - ); - state.videoControl.secondaryControlsEl.append(state.videoQualityControl.el); + state.videoQualityControl.el = state.el.find('a.quality_control'); state.videoQualityControl.quality = null; if (!onTouchBasedDevice()) { + // REFACTOR: Move qtip config to state.config state.videoQualityControl.el.qtip({ 'position': { 'my': 'top right', @@ -72,6 +70,7 @@ function () { function onQualityChange(value) { this.videoQualityControl.quality = value; + // refactor: Move constants to state.config. if ((value === 'hd720') || (value === 'hd1080') || (value === 'highres')) { this.videoQualityControl.el.addClass('active'); } else { @@ -86,6 +85,7 @@ function () { _ref = this.videoQualityControl.quality; + // refactor: Move constants to state.config. if ((_ref === 'hd720') || (_ref === 'hd1080') || (_ref === 'highres')) { newQuality = 'large'; } else { diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_speed_control.js b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_speed_control.js index d9a79b2bc3..2c8d8ca9b7 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_speed_control.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_speed_control.js @@ -37,27 +37,19 @@ function () { function renderElements(state) { state.videoSpeedControl.speeds = state.speeds; - state.videoSpeedControl.el = $( - '
      ' + - '' + - '

      Speed

      ' + - '

      ' + - '      ' + - '
        ' + - '
        ' - ); + state.videoSpeedControl.el = state.el.find('div.speeds'); state.videoSpeedControl.videoSpeedsEl = state.videoSpeedControl.el.find('.video_speeds'); state.videoControl.secondaryControlsEl.prepend(state.videoSpeedControl.el); $.each(state.videoSpeedControl.speeds, function(index, speed) { - var link; - - link = $('').attr({ + // REFACTOR: Move all HTML into one function call. + var link = $('').attr({ 'href': '#' }).html('' + speed + 'x'); + // REFACTOR: Use jQuery .data() state.videoSpeedControl.videoSpeedsEl.prepend($('
      1. ').attr('data-speed', speed).html(link)); }); @@ -76,6 +68,7 @@ function () { $(this).toggleClass('open'); }); } else { + // REFACTOR: Chain. state.videoSpeedControl.el.on('mouseenter', function() { $(this).addClass('open'); }); @@ -98,6 +91,7 @@ function () { // *************************************************************** function setSpeed(speed) { + // REFACTOR: Use chaining. this.videoSpeedControl.videoSpeedsEl.find('li').removeClass('active'); this.videoSpeedControl.videoSpeedsEl.find("li[data-speed='" + speed + "']").addClass('active'); this.videoSpeedControl.el.find('p.active').html('' + speed + 'x'); @@ -106,10 +100,12 @@ function () { function changeVideoSpeed(event) { event.preventDefault(); + // REFACTOR: Cache parent el. if (!$(event.target).parent().hasClass('active')) { this.videoSpeedControl.currentSpeed = $(event.target).parent().data('speed'); this.videoSpeedControl.setSpeed( + // To meet the API expected format. parseFloat(this.videoSpeedControl.currentSpeed).toFixed(2).replace(/\.00$/, '.0') ); @@ -117,6 +113,7 @@ function () { } } + // REFACTOR. function reRender(params /*newSpeeds, currentSpeed*/) { var _this; diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_volume_control.js b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_volume_control.js index ed62b3c7b7..3d979a0d1a 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_volume_control.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/display/video_volume_control.js @@ -34,14 +34,7 @@ function () { // 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) { - state.videoVolumeControl.el = $( - '
        ' + - '' + - '
        ' + - '
        ' + - '
        ' + - '
        ' - ); + state.videoVolumeControl.el = state.el.find('div.volume'); state.videoVolumeControl.buttonEl = state.videoVolumeControl.el.find('a'); state.videoVolumeControl.volumeSliderEl = state.videoVolumeControl.el.find('.volume-slider'); @@ -51,6 +44,7 @@ function () { // Figure out what the current volume is. Set it up so that muting/unmuting works correctly. // If no information about volume level could be retrieved, then we will use the default // 100 level (full volume). + // REFACTOR: Remove unnecessary checks. state.videoVolumeControl.currentVolume = parseInt($.cookie('video_player_volume_level'), 10); state.videoVolumeControl.previousVolume = 100; if ( diff --git a/common/lib/xmodule/xmodule/js/src/videoalpha/main.js b/common/lib/xmodule/xmodule/js/src/videoalpha/main.js index edd383250f..ac29e5f16d 100644 --- a/common/lib/xmodule/xmodule/js/src/videoalpha/main.js +++ b/common/lib/xmodule/xmodule/js/src/videoalpha/main.js @@ -1,5 +1,7 @@ (function (requirejs, require, define) { +// REFACTOR. Build JS doc. Add docs on how to build docs. + // Main module. require( [ @@ -49,7 +51,10 @@ function ( VideoProgressSlider(state); VideoVolumeControl(state); VideoSpeedControl(state); - VideoCaption(state); + + if (state.config.show_captions) { + VideoCaption(state); + } }; }); diff --git a/lms/envs/common.py b/lms/envs/common.py index 6c64cf1d90..4f796d8534 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -35,7 +35,7 @@ from .discussionsettings import * PLATFORM_NAME = "edX" COURSEWARE_ENABLED = True -ENABLE_JASMINE = False +ENABLE_JASMINE = True PERFSTATS = False diff --git a/lms/templates/videoalpha.html b/lms/templates/videoalpha.html index f6ed6bfd27..2af0896fdf 100644 --- a/lms/templates/videoalpha.html +++ b/lms/templates/videoalpha.html @@ -34,8 +34,42 @@
        -
        +
        +
        +
        +
          +
          • +
        • 0:00 / 0:00
          • +
        +
        +
        + +

        Speed

        +

        +         +
          +
          +
          + +
          +
          +
          +
          + Fill Browser + HD + + % if show_captions == 'true': + Captions + % endif +
          +
          +
          + + % if show_captions == 'true': +
            + % endif +