From 55d84d34b854a00a15f04a98920c2e64d201c96c Mon Sep 17 00:00:00 2001 From: Chris Rodriguez Date: Tue, 10 May 2016 14:58:02 -0400 Subject: [PATCH] AC-454 adding visual clarify for selected menu options --- .../xmodule/xmodule/css/video/display.scss | 2 + .../js/fixtures/video_yt_multiple.html | 9 +- .../xmodule/xmodule/js/karma_xmodule.conf.js | 9 +- .../xmodule/xmodule/js/spec/main_requirejs.js | 40 +++- .../js/spec/video/video_caption_spec.js | 4 +- .../js/spec/video/video_speed_control_spec.js | 14 +- .../js/src/video/08_video_speed_control.js | 79 +++++-- .../xmodule/js/src/video/09_video_caption.js | 197 ++++++++++++------ lms/static/karma_lms_coffee.conf.js | 23 +- .../lms/js/spec/main_requirejs_coffee.js | 38 +++- 10 files changed, 298 insertions(+), 117 deletions(-) diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index 116818fb08..df407aee0d 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -535,6 +535,8 @@ .speed-option, .control-lang { + @include border-left($baseline/10 solid rgb(14, 166, 236)); + font-weight: $font-bold; color: rgb(14, 166, 236); // UXPL primary accent } } diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html index d4f5401785..c941c535a6 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html @@ -55,8 +55,9 @@ - -
+
+
+
@@ -108,7 +109,9 @@ -
+
+
+
diff --git a/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js b/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js index 5ee38962a7..3f72dcb7b0 100644 --- a/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js +++ b/common/lib/xmodule/xmodule/js/karma_xmodule.conf.js @@ -20,12 +20,10 @@ var options = { libraryFilesToInclude: [ {pattern: 'common_static/js/vendor/requirejs/require.js', included: true}, {pattern: 'RequireJS-namespace-undefine.js', included: true}, - {pattern: 'spec/main_requirejs.js', included: true}, {pattern: 'common_static/coffee/src/ajax_prefix.js', included: true}, {pattern: 'common_static/common/js/vendor/underscore.js', included: true}, {pattern: 'common_static/common/js/vendor/backbone.js', included: true}, - {pattern: 'common_static/edx-ui-toolkit/js/utils/global-loader.js', included: true}, {pattern: 'common_static/js/vendor/CodeMirror/codemirror.js', included: true}, {pattern: 'common_static/js/vendor/draggabilly.js'}, {pattern: 'common_static/common/js/vendor/jquery.js', included: true}, @@ -50,11 +48,14 @@ var options = { {pattern: 'common_static/js/vendor/jasmine-imagediff.js', included: true}, {pattern: 'common_static/common/js/spec_helpers/jasmine-waituntil.js', included: true}, {pattern: 'common_static/common/js/spec_helpers/jasmine-extensions.js', included: true}, - {pattern: 'common_static/js/vendor/sinon-1.17.0.js', included: true} + {pattern: 'common_static/js/vendor/sinon-1.17.0.js', included: true}, + + {pattern: 'spec/main_requirejs.js', included: true}, ], libraryFiles: [ - {pattern: 'common_static/edx-pattern-library/js/**/*.js'} + {pattern: 'common_static/edx-pattern-library/js/**/*.js'}, + {pattern: 'common_static/edx-ui-toolkit/js/**/*.js'} ], // Make sure the patterns in sourceFiles and specFiles do not match the same file. diff --git a/common/lib/xmodule/xmodule/js/spec/main_requirejs.js b/common/lib/xmodule/xmodule/js/spec/main_requirejs.js index a57497f8c8..d4692469e5 100644 --- a/common/lib/xmodule/xmodule/js/spec/main_requirejs.js +++ b/common/lib/xmodule/xmodule/js/spec/main_requirejs.js @@ -1,4 +1,36 @@ -(function(requirejs) { +(function(requirejs, define) { + 'use strict'; + // We do not wish to bundle common libraries (that may also be used by non-RequireJS code on the page + // into the optimized files. Therefore load these libraries through script tags and explicitly define them. + // Note that when the optimizer executes this code, window will not be defined. + if (window) { + var defineDependency = function (globalName, name, noShim) { + var getGlobalValue = function(name) { + var globalNamePath = name.split('.'), + result = window, + i; + for (i = 0; i < globalNamePath.length; i++) { + result = result[globalNamePath[i]]; + } + return result; + }, + globalValue = getGlobalValue(globalName); + if (globalValue) { + if (noShim) { + define(name, {}); + } + else { + define(name, [], function() { return globalValue; }); + } + } + else { + console.error("Expected library to be included on page, but not found on window object: " + name); + } + }; + defineDependency("jQuery", "jquery"); + defineDependency("jQuery", "jquery-migrate"); + defineDependency("_", "underscore"); + } requirejs.config({ baseUrl: '/base/', paths: { @@ -6,7 +38,8 @@ "modernizr": "common_static/edx-pattern-library/js/modernizr-custom", "afontgarde": "common_static/edx-pattern-library/js/afontgarde", "edxicons": "common_static/edx-pattern-library/js/edx-icons", - "draggabilly": "common_static/js/vendor/draggabilly" + "draggabilly": "common_static/js/vendor/draggabilly", + 'edx-ui-toolkit': 'common_static/edx-ui-toolkit' }, "moment": { exports: "moment" @@ -18,5 +51,4 @@ exports: "AFontGarde" } }); - -}).call(this, RequireJS.requirejs); +}).call(this, RequireJS.requirejs, RequireJS.define); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js index b8cf3065b1..5d85be2551 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_caption_spec.js @@ -266,6 +266,7 @@ expect($('.closed-captions')).toHaveAttrs({ 'lang': 'de' }); + expect(link).toHaveAttr('aria-pressed', 'true'); }); it('when clicking on link with current language', function () { @@ -284,6 +285,7 @@ expect(state.storage.setItem) .not.toHaveBeenCalledWith('language', 'en'); expect($('.langs-list li.is-active').length).toBe(1); + expect(link).toHaveAttr('aria-pressed', 'true'); }); it('open the language toggle on hover', function () { @@ -413,7 +415,7 @@ }); it('show explanation message', function () { - expect($('.subtitles-menu li')).toHaveText( + expect($('.subtitles .subtitles-menu li')).toHaveText( 'Transcript will be displayed when you start playing the video.' ); }); 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 2a8b5e8b67..f73b1f3c59 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 @@ -203,16 +203,18 @@ describe('onSpeedChange', function () { beforeEach(function () { state = jasmine.initializePlayer(); - $('li[data-speed="1.0"]').addClass('is-active'); + $('li[data-speed="1.0"]').addClass('is-active').attr('aria-pressed', 'true'); state.videoSpeedControl.setSpeed(0.75); }); it('set the new speed as active', function () { - expect($('.video-speeds li[data-speed="1.0"]')) - .not.toHaveClass('is-active'); - expect($('.video-speeds li[data-speed="0.75"]')) - .toHaveClass('is-active'); - expect($('.speeds .value')).toHaveHtml('0.75x'); + expect($('li[data-speed="1.0"]')).not.toHaveClass('is-active'); + expect($('li[data-speed="1.0"] .speed-option').attr('aria-pressed')).not.toEqual('true'); + + expect($('li[data-speed="0.75"]')).toHaveClass('is-active'); + expect($('li[data-speed="0.75"] .speed-option').attr('aria-pressed')).toEqual('true'); + + expect($('.speeds .speed-button .value')).toHaveHtml('0.75x'); }); }); 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 986d9ea7ff..b54de6a24a 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 @@ -1,9 +1,10 @@ (function (requirejs, require, define) { "use strict"; define( -'video/08_video_speed_control.js', -['video/00_iterator.js'], -function (Iterator) { +'video/08_video_speed_control.js', [ + 'video/00_iterator.js', + 'edx-ui-toolkit/js/utils/html-utils' +], function (Iterator, HtmlUtils) { /** * Video speed control module. * @exports video/08_video_speed_control.js @@ -95,23 +96,37 @@ function (Iterator) { * Creates any necessary DOM elements, attach them, and set their, * initial configuration. * @param {array} speeds List of speeds available for the player. + * @param {string} currentSpeed The current speed set to the player. */ - render: function (speeds) { + render: function (speeds, currentSpeed) { var speedsContainer = this.speedsContainer, reversedSpeeds = speeds.concat().reverse(), speedsList = $.map(reversedSpeeds, function (speed) { - return [ - '
  • ', - '', - '
  • ' - ].join(''); + return HtmlUtils.interpolateHtml( + HtmlUtils.joinHtml( + HtmlUtils.HTML('
  • '), + HtmlUtils.HTML(''), + HtmlUtils.HTML('
  • ') + ), + { + speed: speed + } + ).toString(); }); - speedsContainer.html(speedsList.join('')); + HtmlUtils.setHtml( + speedsContainer, + HtmlUtils.HTML(speedsList) + ); this.speedLinks = new Iterator(speedsContainer.find('.speed-option')); - this.state.el.find('.secondary-controls').prepend(this.el); + HtmlUtils.prepend( + this.state.el.find('.secondary-controls'), + HtmlUtils.HTML(this.el) + ); + this.setActiveSpeed(currentSpeed); }, /** @@ -216,17 +231,38 @@ function (Iterator) { if (speed !== this.currentSpeed || forceUpdate) { this.speedsContainer .find('li') - .removeClass('is-active') - .siblings("li[data-speed='" + speed + "']") - .addClass('is-active'); + .siblings("li[data-speed='" + speed + "']"); - this.speedButton.find('.value').html(speed + 'x'); + this.speedButton.find('.value').text(speed + 'x'); this.currentSpeed = speed; if (!silent) { this.el.trigger('speedchange', [speed, this.state.speed]); } } + + this.resetActiveSpeed(); + this.setActiveSpeed(speed); + }, + + resetActiveSpeed: function() { + var speedOptions = this.speedsContainer.find('li'); + + $(speedOptions).each(function(index, el) { + $(el).removeClass('is-active') + .find('.speed-option') + .attr('aria-pressed', 'false'); + }); + }, + + setActiveSpeed: function(speed) { + var speedOption = this.speedsContainer.find('li[data-speed="' + speed + '"]'); + + speedOption.addClass('is-active') + .find('.speed-option') + .attr('aria-pressed', 'true'); + + this.speedButton.attr('title', gettext('Video speed: ') + speed + 'x'); }, /** @@ -244,10 +280,13 @@ function (Iterator) { * @param {jquery Event} event */ clickLinkHandler: function (event) { - var speed = $(event.currentTarget).parent().data('speed'); - - this.closeMenu(); + var el = $(event.currentTarget).parent(), + speed = $(el).data('speed'); + + this.resetActiveSpeed(); + this.setActiveSpeed(speed); this.state.videoCommands.execute('speed', speed); + this.closeMenu(true); return false; }, diff --git a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js b/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js index b99a1b1128..dd409e7a91 100644 --- a/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js +++ b/common/lib/xmodule/xmodule/js/src/video/09_video_caption.js @@ -5,11 +5,12 @@ define('video/09_video_caption.js',[ 'video/00_sjson.js', 'video/00_async_process.js', + 'edx-ui-toolkit/js/utils/html-utils', 'draggabilly', 'modernizr', 'afontgarde', 'edxicons' - ], function (Sjson, AsyncProcess, Draggabilly) { + ], function (Sjson, AsyncProcess, HtmlUtils, Draggabilly) { /** * @desc VideoCaption module exports a function. @@ -80,47 +81,53 @@ renderElements: function () { var languages = this.state.config.transcriptLanguages; - var langTemplate = [ - '
    ', - '', - '', - '' - ].join(''); + HtmlUtils.HTML('">'), + HtmlUtils.HTML(''), + HtmlUtils.HTML(''), + HtmlUtils.HTML(''), + HtmlUtils.HTML(''), + HtmlUtils.HTML(''), + HtmlUtils.HTML('
    '), + HtmlUtils.HTML(')') + ); - var template = [ - '
    ', - '

    ', - '
      ', - '
      ' - ].join(''); + var subtitlesHtml = HtmlUtils.interpolateHtml( + HtmlUtils.joinHtml( + HtmlUtils.HTML('
      '), + HtmlUtils.HTML('

      '), + HtmlUtils.HTML('
        '), + HtmlUtils.HTML('
        ') + ), + { + courseId: this.state.id, + courseLang: this.state.lang + } + ); this.loaded = false; - this.subtitlesEl = $(template); + this.subtitlesEl = $(HtmlUtils.ensureHtml(subtitlesHtml).toString()); this.subtitlesMenuEl = this.subtitlesEl.find('.subtitles-menu'); - this.container = $(langTemplate); + this.container = $(HtmlUtils.ensureHtml(langHtml).toString()); this.captionControlEl = this.container.find('.toggle-captions'); this.captionDisplayEl = this.state.el.find('.closed-captions'); this.transcriptControlEl = this.container.find('.toggle-transcript'); @@ -542,15 +549,26 @@ } } else { if (state.isTouch) { - self.subtitlesEl.find('.subtitles-menu') - .text(gettext('Transcript will be displayed when you start playing the video.')) // jshint ignore: line - .wrapInner('
      1. '); + HtmlUtils.setHtml( + self.subtitlesEl.find('.subtitles-menu'), + HtmlUtils.joinHtml( + HtmlUtils.HTML('
      2. '), + gettext('Transcript will be displayed when you start playing the video.'), + HtmlUtils.HTML('
      3. ') + ) + ); } else { self.renderCaption(start, captions); } self.hideCaptions(state.hide_captions, false); - self.state.el.find('.video-wrapper').after(self.subtitlesEl); - self.state.el.find('.secondary-controls').append(self.container); + HtmlUtils.append( + self.state.el.find('.video-wrapper').parent(), + HtmlUtils.HTML(self.subtitlesEl) + ); + HtmlUtils.append( + self.state.el.find('.secondary-controls'), + HtmlUtils.HTML(self.container) + ); self.bindHandlers(); } @@ -630,9 +648,11 @@ onResize: function () { this.subtitlesEl .find('.spacing').first() - .height(this.topSpacingHeight()).end() + .height(this.topSpacingHeight()); + + this.subtitlesEl .find('.spacing').last() - .height(this.bottomSpacingHeight()); + .height(this.bottomSpacingHeight()); this.scrollCaption(); this.setSubtitlesHeight(); @@ -649,8 +669,9 @@ renderLanguageMenu: function (languages) { var self = this, state = this.state, - menu = $('