AC-454 adding visual clarify for selected menu options

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
                 }
               }

common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html

@@ -55,8 +55,9 @@
               </div>
             </section>
           </article>
-
-          <ol class="subtitles"><li></li></ol>
+          <div class="subtitles">
+              <ol class="subtitles-menu"><li></li></ol>
+          </div>
         </div>
       </div>
     </div>
@@ -108,7 +109,9 @@
             </section>
           </article>
 
-          <ol class="subtitles"><li></li></ol>
+          <div class="subtitles">
+              <ol class="subtitles-menu"><li></li></ol>
+          </div>
         </div>
       </div>
     </div>

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.

common/lib/xmodule/xmodule/js/spec/main_requirejs.js

-(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);

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.'
                     );
                 });

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');
             });
         });
 

common/lib/xmodule/xmodule/js/src/video/08_video_speed_control.js

 (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,38 @@ 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 [
-                        '<li data-speed="', speed, '">',
-                            '<button class="control speed-option" tabindex="-1">',
-                                speed, 'x',
-                            '</button>',
-                        '</li>'
-                    ].join('');
+                    return HtmlUtils.interpolateHtml(
+                        HtmlUtils.HTML(
+                            [
+                                '<li data-speed="{speed}">',
+                                    '<button class="control speed-option" tabindex="-1" aria-pressed="false">',
+                                        '{speed}x',
+                                    '</button>',
+                                '</li>'
+                            ].join('')
+                        ),
+                        {
+                            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 +232,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 +281,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;
         },