From f0492b3bb11123a9b546e1238625321d6559fe66 Mon Sep 17 00:00:00 2001 From: polesye Date: Fri, 4 Apr 2014 11:19:42 +0300 Subject: [PATCH] Refactor speed control. --- .../contentstore/features/video-editor.py | 2 +- .../xmodule/xmodule/css/video/display.scss | 19 +- .../xmodule/xmodule/js/fixtures/video.html | 8 +- .../xmodule/js/fixtures/video_all.html | 8 +- .../js/fixtures/video_yt_multiple.html | 24 +- .../xmodule/js/spec/video/iterator_spec.js | 103 +++ .../js/spec/video/video_caption_spec.js | 8 +- .../js/spec/video/video_player_spec.js | 2 - .../js/spec/video/video_speed_control_spec.js | 143 ++-- .../xmodule/js/src/video/00_iterator.js | 101 +++ .../xmodule/js/src/video/01_initialize.js | 11 +- .../xmodule/js/src/video/03_video_player.js | 18 +- .../js/src/video/07_video_volume_control.js | 2 - .../js/src/video/08_video_speed_control.js | 631 +++++++++--------- .../xmodule/js/src/video/09_video_caption.js | 10 +- .../xmodule/video_module/video_module.py | 1 + lms/djangoapps/courseware/features/video.py | 14 +- lms/templates/video.html | 8 +- 18 files changed, 643 insertions(+), 470 deletions(-) create mode 100644 common/lib/xmodule/xmodule/js/spec/video/iterator_spec.js create mode 100644 common/lib/xmodule/xmodule/js/src/video/00_iterator.js diff --git a/cms/djangoapps/contentstore/features/video-editor.py b/cms/djangoapps/contentstore/features/video-editor.py index a7974f020f..6872553cb6 100644 --- a/cms/djangoapps/contentstore/features/video-editor.py +++ b/cms/djangoapps/contentstore/features/video-editor.py @@ -102,7 +102,7 @@ def choose_new_lang(lang_code): def open_menu(menu): - world.browser.execute_script("$('{selector}').parent().addClass('open')".format( + world.browser.execute_script("$('{selector}').parent().addClass('is-opened')".format( selector=VIDEO_MENUS[menu] )) diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index 8690ed43ae..ce18abe093 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -317,7 +317,10 @@ div.video { div.secondary-controls { float: right; - div.speeds>a, div.volume>a, a.add-fullscreen, a.quality_control, + a.speed-button, + div.volume > a, + a.add-fullscreen, + a.quality_control, a.hide-subtitles { // overflow is used to bypass Firefox CSS :focus outline bug // http://johndoesdesign.com/blog/2012/css/firefox-and-its-css-focus-outline-bug/ @@ -334,7 +337,7 @@ div.video { float: left; position: relative; - &.open { + &.is-opened { .menu { display: block; opacity: 1; @@ -377,7 +380,7 @@ div.video { } } - &.active{ + &.is-active{ a { font-weight: bold; } @@ -393,8 +396,8 @@ div.video { } div.speeds { - &.open { - & > a { + &.is-opened { + .speed-button { background-image: url('../images/open-arrow.png'); } } @@ -407,7 +410,7 @@ div.video { } } - & > a { + .speed-button { @extend %video-button; @include clearfix(); background-image: url('../images/closed-arrow.png'); @@ -421,7 +424,7 @@ div.video { width: 60px; } - h3 { + .label { float: left; font-size: em(14); font-weight: normal; @@ -436,7 +439,7 @@ div.video { } } - p.active { + .value { float: left; font-weight: bold; margin-bottom: 0; diff --git a/common/lib/xmodule/xmodule/js/fixtures/video.html b/common/lib/xmodule/xmodule/js/fixtures/video.html index c61e79d8c4..2a9c31eb43 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video.html @@ -41,11 +41,11 @@
- -

Speed

-

+ + Speed + -
    +
      diff --git a/common/lib/xmodule/xmodule/js/fixtures/video_all.html b/common/lib/xmodule/xmodule/js/fixtures/video_all.html index f0228b44a2..fd986b9437 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_all.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_all.html @@ -44,11 +44,11 @@
      - > -

      Speed

      -

      +       + Speed + -
        +
          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 83dfdc9f0e..d7026d654a 100644 --- a/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html +++ b/common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html @@ -41,11 +41,11 @@
          - -

          Speed

          -

          +           + Speed + -
            +
              @@ -108,11 +108,11 @@
              - -

              Speed

              -

              +               + Speed + -
                +
                  @@ -173,11 +173,11 @@
                  - -

                  Speed

                  -

                  +                   + Speed + -
                    +
                      diff --git a/common/lib/xmodule/xmodule/js/spec/video/iterator_spec.js b/common/lib/xmodule/xmodule/js/spec/video/iterator_spec.js new file mode 100644 index 0000000000..d3fe711588 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/spec/video/iterator_spec.js @@ -0,0 +1,103 @@ +(function (require) { +require( +['video/00_iterator.js'], +function (Iterator) { + describe('Iterator', function () { + var list = ['a', 'b', 'c', 'd', 'e'], + iterator; + + beforeEach(function() { + iterator = new Iterator(list); + }); + + it('size contains correct list length', function () { + expect(iterator.size).toBe(list.length); + expect(iterator.lastIndex).toBe(list.length - 1); + }); + + describe('next', function () { + describe('with passed `index`', function () { + it('returns next item in the list', function () { + expect(iterator.next(2)).toBe('d'); + expect(iterator.next(0)).toBe('b'); + }); + + it('returns first item if index equal last item', function () { + expect(iterator.next(4)).toBe('a'); + }); + + it('returns next item if index is not valid', function () { + expect(iterator.next(-4)).toBe('b'); // index < 0 + expect(iterator.next(100)).toBe('c'); // index > size + expect(iterator.next('99')).toBe('d'); // incorrect Type + }); + }); + + describe('without passed `index`', function () { + it('returns next item in the list', function () { + expect(iterator.next()).toBe('b'); + expect(iterator.next()).toBe('c'); + }); + + it('returns first item if index equal last item', function () { + expect(iterator.next()).toBe('b'); + expect(iterator.next()).toBe('c'); + expect(iterator.next()).toBe('d'); + expect(iterator.next()).toBe('e'); + expect(iterator.next()).toBe('a'); + }); + }); + }); + + describe('prev', function () { + describe('with passed `index`', function () { + it('returns previous item in the list', function () { + expect(iterator.prev(3)).toBe('c'); + expect(iterator.prev(1)).toBe('a'); + }); + + it('returns last item if index equal first item', function () { + expect(iterator.prev(0)).toBe('e'); + }); + + it('returns previous item if index is not valid', function () { + expect(iterator.prev(-4)).toBe('e'); // index < 0 + expect(iterator.prev(100)).toBe('d'); // index > size + expect(iterator.prev('99')).toBe('c'); // incorrect Type + }); + }); + + describe('without passed `index`', function () { + it('returns previous item in the list', function () { + expect(iterator.prev()).toBe('e'); + expect(iterator.prev()).toBe('d'); + }); + + it('returns previous item if index equal first item', function () { + expect(iterator.prev()).toBe('e'); + }); + }); + }); + + it('returns last item in the list', function () { + expect(iterator.last()).toBe('e'); + }); + + it('returns first item in the list', function () { + expect(iterator.first()).toBe('a'); + }); + + it('isEnd works correctly', function () { + expect(iterator.isEnd()).toBeFalsy(); + iterator.next(); // => index 1 + expect(iterator.isEnd()).toBeFalsy(); + iterator.next(); // => index 2 + expect(iterator.isEnd()).toBeFalsy(); + iterator.next(); // => index 3 + expect(iterator.isEnd()).toBeFalsy(); + iterator.next(); // => index 4 == last + expect(iterator.isEnd()).toBeTruthy(); + }); + }); +}); +}(RequireJS.require)); 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 3bc7b5c858..fdf854ab34 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 @@ -180,7 +180,7 @@ expect(state.lang).toBe('de'); expect(state.storage.setItem) .toHaveBeenCalledWith('language', 'de'); - expect($('.langs-list li.active').length).toBe(1); + expect($('.langs-list li.is-active').length).toBe(1); }); it('when clicking on link with current language', function () { @@ -198,15 +198,15 @@ expect(state.lang).toBe('en'); expect(state.storage.setItem) .not.toHaveBeenCalledWith('language', 'en'); - expect($('.langs-list li.active').length).toBe(1); + expect($('.langs-list li.is-active').length).toBe(1); }); it('open the language toggle on hover', function () { state = jasmine.initializePlayer(); $('.lang').mouseenter(); - expect($('.lang')).toHaveClass('open'); + expect($('.lang')).toHaveClass('is-opened'); $('.lang').mouseleave(); - expect($('.lang')).not.toHaveClass('open'); + expect($('.lang')).not.toHaveClass('is-opened'); }); }); diff --git a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js index 6c6685e4fc..c0d48bd95d 100644 --- a/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js +++ b/common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js @@ -52,8 +52,6 @@ function (VideoPlayer) { it('create video speed control', function () { expect(state.videoSpeedControl).toBeDefined(); expect(state.videoSpeedControl.el).toHaveClass('speeds'); - expect(state.videoSpeedControl.speeds) - .toEqual([ '0.75', '1.0', '1.25', '1.50' ]); expect(state.speed).toEqual('1.50'); }); 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 73be6ad964..c449fe011d 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 @@ -21,26 +21,24 @@ }); it('add the video speed control to player', function () { - var li, secondaryControls; - - secondaryControls = $('.secondary-controls'); - li = secondaryControls.find('.video_speeds li'); + var secondaryControls = $('.secondary-controls'), + li = secondaryControls.find('.video-speeds li'); expect(secondaryControls).toContain('.speeds'); - expect(secondaryControls).toContain('.video_speeds'); - expect(secondaryControls.find('p.active').text()) + expect(secondaryControls).toContain('.video-speeds'); + expect(secondaryControls.find('.value').text()) .toBe('1.50x'); - expect(li.filter('.active')).toHaveData( + expect(li.filter('.is-active')).toHaveData( 'speed', state.videoSpeedControl.currentSpeed ); - expect(li.length).toBe(state.videoSpeedControl.speeds.length); + expect(li.length).toBe(state.speeds.length); $.each(li.toArray().reverse(), function (index, link) { expect($(link)).toHaveData( - 'speed', state.videoSpeedControl.speeds[index] + 'speed', state.speeds[index] ); expect($(link).find('a').text()).toBe( - state.videoSpeedControl.speeds[index] + 'x' + state.speeds[index] + 'x' ); }); }); @@ -68,7 +66,7 @@ }); describe('when running on non-touch based device', function () { - var speedControl, speedEntries, + var speedControl, speedEntries, speedButton, KEY = $.ui.keyCode, keyPressEvent = function(key) { @@ -103,17 +101,18 @@ beforeEach(function () { state = jasmine.initializePlayer(); - speedControl = $('div.speeds'); - speedEntries = speedControl.children('a'); - spyOn($.fn, 'focus').andCallThrough(); + speedControl = $('.speeds'); + speedButton = $('.speed-button'); + speedsContainer = $('.video-speeds'); + speedEntries = speedsContainer.find('a'); }); it('open/close the speed menu on mouseenter/mouseleave', function () { speedControl.mouseenter(); - expect(speedControl).toHaveClass('open'); + expect(speedControl).toHaveClass('is-opened'); speedControl.mouseleave(); - expect(speedControl).not.toHaveClass('open'); + expect(speedControl).not.toHaveClass('is-opened'); }); it('do not close the speed menu on mouseleave if a speed ' + @@ -121,64 +120,62 @@ // Open speed meenu. Focus is on last speed entry. speedControl.trigger(keyPressEvent(KEY.ENTER)); speedControl.mouseenter().mouseleave(); - expect(speedControl).toHaveClass('open'); + expect(speedControl).toHaveClass('is-opened'); }); it('close the speed menu on click', function () { speedControl.mouseenter().click(); - expect(speedControl).not.toHaveClass('open'); + expect(speedControl).not.toHaveClass('is-opened'); }); it('close the speed menu on outside click', function () { speedControl.trigger(keyPressEvent(KEY.ENTER)); $(window).click(); - expect(speedControl).not.toHaveClass('open'); + expect(speedControl).not.toHaveClass('is-opened'); }); it('open the speed menu on ENTER keydown', function () { speedControl.trigger(keyPressEvent(KEY.ENTER)); - expect(speedControl).toHaveClass('open'); - expect(speedEntries.last().focus).toHaveBeenCalled(); + expect(speedControl).toHaveClass('is-opened'); + expect(speedEntries.last()).toBeFocused(); }); it('open the speed menu on SPACE keydown', function () { speedControl.trigger(keyPressEvent(KEY.SPACE)); - expect(speedControl).toHaveClass('open'); - expect(speedEntries.last().focus).toHaveBeenCalled(); + expect(speedControl).toHaveClass('is-opened'); + expect(speedEntries.last()).toBeFocused(); }); it('open the speed menu on UP keydown', function () { speedControl.trigger(keyPressEvent(KEY.UP)); - expect(speedControl).toHaveClass('open'); - expect(speedEntries.last().focus).toHaveBeenCalled(); + expect(speedControl).toHaveClass('is-opened'); + expect(speedEntries.last()).toBeFocused(); }); it('close the speed menu on ESCAPE keydown', function () { speedControl.trigger(keyPressEvent(KEY.ESCAPE)); - expect(speedControl).not.toHaveClass('open'); + expect(speedControl).not.toHaveClass('is-opened'); }); it('UP and DOWN keydown function as expected on speed entries', function () { // Iterate through list in both directions and check if // things wrap up correctly. - var lastEntry = speedEntries.length-1, i; + var lastEntry = speedEntries.length-1, + speed_0_75 = speedEntries.filter(':contains("0.75x")'), + speed_1_0 = speedEntries.filter(':contains("1.0x")'); // First open menu speedControl.trigger(keyPressEvent(KEY.UP)); + expect(speed_0_75).toBeFocused(); // Iterate with UP key until we have looped. - for (i = lastEntry; i >= 0; i--) { - speedEntries.eq(i).trigger(keyPressEvent(KEY.UP)); - } + speed_0_75.trigger(keyPressEvent(KEY.UP)); + expect(speed_1_0).toBeFocused(); - // Iterate with DOWN key until we have looped. - for (i = 0; i <= lastEntry; i++) { - speedEntries.eq(i).trigger(keyPressEvent(KEY.DOWN)); - } - // Test if each element has been called twice. - expect($.fn.focus.calls.length) - .toEqual(2*speedEntries.length); + // // Iterate with DOWN key until we have looped. + speed_1_0.trigger(keyPressEvent(KEY.DOWN)); + expect(speed_0_75).toBeFocused(); }); it('ESC keydown on speed entry closes menu', function () { @@ -188,8 +185,8 @@ // Menu is closed and focus has been returned to speed // control. - expect(speedControl).not.toHaveClass('open'); - expect(speedControl.focus).toHaveBeenCalled(); + expect(speedControl).not.toHaveClass('is-opened'); + expect(speedButton).toBeFocused(); }); it('ENTER keydown on speed entry selects speed and closes menu', @@ -197,15 +194,15 @@ // First open menu. speedControl.trigger(keyPressEvent(KEY.UP)); // Focus on 1.50x speed - speedEntries.eq(1).focus(); - speedEntries.eq(1).trigger(keyPressEvent(KEY.ENTER)); + speedEntries.eq(0).focus(); + speedEntries.eq(0).trigger(keyPressEvent(KEY.ENTER)); // Menu is closed, focus has been returned to speed // control and video speed is 1.50x. - expect(speedControl.focus).toHaveBeenCalled(); - expect($('.video_speeds li[data-speed="1.50"]')) - .toHaveClass('active'); - expect($('.speeds p.active')).toHaveHtml('1.50x'); + expect(speedButton).toBeFocused(); + expect($('.video-speeds li[data-speed="1.50"]')) + .toHaveClass('is-active'); + expect($('.speeds .value')).toHaveHtml('1.50x'); }); it('SPACE keydown on speed entry selects speed and closes menu', @@ -213,39 +210,15 @@ // First open menu. speedControl.trigger(keyPressEvent(KEY.UP)); // Focus on 1.50x speed - speedEntries.eq(1).focus(); - speedEntries.eq(1).trigger(keyPressEvent(KEY.SPACE)); + speedEntries.eq(0).focus(); + speedEntries.eq(0).trigger(keyPressEvent(KEY.SPACE)); // Menu is closed, focus has been returned to speed // control and video speed is 1.50x. - expect(speedControl.focus).toHaveBeenCalled(); - expect($('.video_speeds li[data-speed="1.50"]')) - .toHaveClass('active'); - expect($('.speeds p.active')).toHaveHtml('1.50x'); - }); - - it('TAB + SHIFT keydown on speed entry closes menu and gives ' + - 'focus to Play/Pause control', function () { - // First open menu. Focus is on last speed entry. - speedControl.trigger(keyPressEvent(KEY.UP)); - speedEntries.last().trigger(tabBackPressEvent()); - - // Menu is closed and focus has been given to Play/Pause - // control. - expect(state.videoControl.playPauseEl.focus) - .toHaveBeenCalled(); - }); - - it('TAB keydown on speed entry closes menu and gives focus ' + - 'to Volume control', function () { - // First open menu. Focus is on last speed entry. - speedControl.trigger(keyPressEvent(KEY.UP)); - speedEntries.last().trigger(tabForwardPressEvent()); - - // Menu is closed and focus has been given to Volume - // control. - expect(state.videoVolumeControl.buttonEl.focus) - .toHaveBeenCalled(); + expect(speedButton).toBeFocused(); + expect($('.video-speeds li[data-speed="1.50"]')) + .toHaveClass('is-active'); + expect($('.speeds .value')).toHaveHtml('1.50x'); }); }); }); @@ -261,14 +234,16 @@ beforeEach(function () { state = jasmine.initializePlayer(); state.videoSpeedControl.setSpeed(1.0); - spyOn(state.videoPlayer, 'onSpeedChange').andCallThrough(); + spyOn($.fn, 'trigger').andCallThrough(); $('li[data-speed="0.75"] a').click(); }); it('trigger speedChange event', function () { - expect(state.videoPlayer.onSpeedChange).toHaveBeenCalled(); - expect(state.videoSpeedControl.currentSpeed).toEqual(0.75); + expect($.fn.trigger.mostRecentCall.args[0]).toEqual( + 'speedchange', ['0.75'] + ); + expect(state.videoSpeedControl.currentSpeed).toEqual('0.75'); }); }); }); @@ -276,16 +251,16 @@ describe('onSpeedChange', function () { beforeEach(function () { state = jasmine.initializePlayer(); - $('li[data-speed="1.0"] a').addClass('active'); + $('li[data-speed="1.0"]').addClass('is-active'); state.videoSpeedControl.setSpeed(0.75); }); it('set the new speed as active', function () { - expect($('.video_speeds li[data-speed="1.0"]')) - .not.toHaveClass('active'); - expect($('.video_speeds li[data-speed="0.75"]')) - .toHaveClass('active'); - expect($('.speeds p.active')).toHaveHtml('0.75x'); + 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'); }); }); }); diff --git a/common/lib/xmodule/xmodule/js/src/video/00_iterator.js b/common/lib/xmodule/xmodule/js/src/video/00_iterator.js new file mode 100644 index 0000000000..e77b301077 --- /dev/null +++ b/common/lib/xmodule/xmodule/js/src/video/00_iterator.js @@ -0,0 +1,101 @@ +(function (define) { +define( +'video/00_iterator.js', +[], +function() { +"use strict"; + /** + * @desc Provides convenient way to work with iterate able data. + * + * @param {array} list Array to be iterated. + * + */ + var Iterator = function (list) { + this.list = list; + this.index = 0; + this.size = this.list.length; + this.lastIndex = this.list.length - 1; + }; + + Iterator.prototype = { + + /** + * @desc Returns `true` if current index is valid for the iterator. + * + * @access protected + * @returns {boolean} + * + */ + _isValid: function (index) { + return _.isNumber(index) && index < this.size && index >= 0; + }, + + /** + * @desc Returns next element. + * + * @param {number} index If set, updates current index of iterator. + * @returns {DOM element} + * + */ + next: function (index) { + if (!(this._isValid(index))) { + index = this.index; + } + + this.index = (index >= this.lastIndex) ? 0: index + 1; + + return this.list[this.index]; + }, + + /** + * @desc Returns previous element. + * + * @param {number} index If set, updates current index of iterator. + * @returns {DOM element} + * + */ + prev: function (index) { + if (!(this._isValid(index))) { + index = this.index; + } + + this.index = (index < 1) ? this.lastIndex: index - 1; + + return this.list[this.index]; + }, + + /** + * @desc Returns last element in the list. + * + * @returns {DOM element} + * + */ + last: function () { + return this.list[this.lastIndex]; + }, + + /** + * @desc Returns first element in the list. + * + * @returns {DOM element} + * + */ + first: function () { + return this.list[0]; + }, + + /** + * @desc Returns `true` if current position is last for the iterator. + * + * @returns {boolean} + * + */ + isEnd: function () { + return this.index === this.lastIndex; + } + }; + + return Iterator; +}); +}(RequireJS.define)); + diff --git a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js index 79a1d76af0..48c789dc49 100644 --- a/common/lib/xmodule/xmodule/js/src/video/01_initialize.js +++ b/common/lib/xmodule/xmodule/js/src/video/01_initialize.js @@ -74,6 +74,7 @@ function (VideoPlayer, VideoStorage) { saveState: saveState, setPlayerMode: setPlayerMode, setSpeed: setSpeed, + speedToString: speedToString, trigger: trigger, youtubeId: youtubeId }, @@ -519,9 +520,9 @@ function (VideoPlayer, VideoStorage) { } this.lang = this.config.transcriptLanguage; - this.speed = Number( + this.speed = this.speedToString( this.config.speed || this.config.generalSpeed - ).toFixed(2).replace(/\.00$/, '.0'); + ); if (!(_parseYouTubeIDs(this))) { @@ -630,7 +631,7 @@ function (VideoPlayer, VideoStorage) { var speed; video = video.split(/:/); - speed = parseFloat(video[0]).toFixed(2).replace(/\.00$/, '.0'); + speed = _this.speedToString(video[0]); _this.videos[speed] = video[1]; }); @@ -844,6 +845,10 @@ function (VideoPlayer, VideoStorage) { return this.getPlayerMode() === 'html5'; } + function speedToString(speed) { + return parseFloat(speed).toFixed(2).replace(/\.00$/, '.0'); + } + function getCurrentLanguage() { var keys = _.keys(this.config.transcriptLanguages); diff --git a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js index 8623f67045..42086fb4c0 100644 --- a/common/lib/xmodule/xmodule/js/src/video/03_video_player.js +++ b/common/lib/xmodule/xmodule/js/src/video/03_video_player.js @@ -167,6 +167,7 @@ function (HTML5Video, Resizer) { dfd.resolve(); } } + function _updateVcrAndRegion(state, isYoutube) { var update = function (state) { var duration = state.videoPlayer.duration(), @@ -384,8 +385,6 @@ function (HTML5Video, Resizer) { this.setSpeed(newSpeed, true); this.videoPlayer.setPlaybackRate(newSpeed); - this.el.trigger('speedchange', arguments); - this.saveState(true, { speed: newSpeed }); } @@ -522,6 +521,10 @@ function (HTML5Video, Resizer) { dfd.resolve(); + this.el.on('speedchange', function (event, speed) { + _this.videoPlayer.onSpeedChange(speed); + }); + this.videoPlayer.log('load_video'); availablePlaybackRates = this.videoPlayer.player @@ -590,21 +593,14 @@ function (HTML5Video, Resizer) { _this.speeds.push(key); }); - this.trigger( - 'videoSpeedControl.reRender', - { - newSpeeds: this.speeds, - currentSpeed: this.speed - } - ); this.setSpeed(this.speed); - this.trigger('videoSpeedControl.setSpeed', this.speed); + this.el.trigger('speed:render', [this.speeds, this.speed]); } } if (this.isFlashMode()) { this.setSpeed(this.speed); - this.trigger('videoSpeedControl.setSpeed', this.speed); + this.el.trigger('speed:set', [this.speed]); } if (this.isHtml5Mode()) { diff --git a/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js b/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js index 291743d516..f4e8920b29 100644 --- a/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js +++ b/common/lib/xmodule/xmodule/js/src/video/07_video_volume_control.js @@ -62,8 +62,6 @@ function () { previousVolume = 100, slider, buttonStr, volumeSliderHandleEl; - state.videoControl.secondaryControlsEl.prepend(element); - if (!isFinite(currentVolume)) { currentVolume = 100; } 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 731ef1e987..edb0e321e1 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,354 +1,347 @@ (function (requirejs, require, define) { - -// VideoSpeedControl module. define( 'video/08_video_speed_control.js', -[], -function () { - - // VideoSpeedControl() function - what this module "exports". - return function (state) { - var dfd = $.Deferred(); - - if (state.isTouch) { - // iOS doesn't support speed change - state.el.find('div.speeds').remove(); - dfd.resolve(); - return dfd.promise(); +['video/00_iterator.js'], +function (Iterator) { + "use strict"; + var SpeedControl = function (state) { + if (!(this instanceof SpeedControl)) { + return new SpeedControl(state); } - state.videoSpeedControl = {}; + this.state = state; + this.state.videoSpeedControl = this; + this.initialize(); - _initialize(state); - dfd.resolve(); - - if (state.videoType === 'html5' && !(_checkPlaybackRates())) { - console.log( - '[Video info]: HTML5 mode - playbackRate is not supported.' - ); - - _hideSpeedControl(state); - } - - return dfd.promise(); + return $.Deferred().resolve().promise(); }; - // *************************************************************** - // Private functions start here. - // *************************************************************** + SpeedControl.prototype = { + /** + * @desc Initializes the module. + * + */ + initialize: function () { + var state = this.state; - function _initialize(state) { - _makeFunctionsPublic(state); - _renderElements(state); - _bindHandlers(state); - } + this.el = state.el.find('.speeds'); + this.speedsContainer = this.el.find('.video-speeds'); + this.speedButton = this.el.find('.speed-button'); - // function _makeFunctionsPublic(state) - // - // Functions which will be accessible via 'state' object. When called, - // these functions will get the 'state' object as a context. - function _makeFunctionsPublic(state) { - var methodsDict = { - changeVideoSpeed: changeVideoSpeed, - reRender: reRender, - setSpeed: setSpeed - }; - - state.bindTo(methodsDict, state.videoSpeedControl, 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. - function _renderElements(state) { - state.videoSpeedControl.speeds = state.speeds; - - 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 = '' + speed + 'x'; - - state.videoSpeedControl.videoSpeedsEl - .prepend( - $('
                    1. ' + link + '
                      2. ') + if (!this.isPlaybackRatesSupported(state)) { + this.el.remove(); + console.log( + '[Video info]: playbackRate is not supported.' ); - }); - state.videoSpeedControl.setSpeed(state.speed); - } - - /** - * @desc Check if playbackRate supports by browser. - * - * @type {function} - * @access private - * - * @param {object} state The object containg the state of the video player. - * All other modules, their parameters, public variables, etc. are - * available via this object. - * - * @this {object} The global window object. - * - * @returns {Boolean} - * true: Browser support playbackRate functionality. - * false: Browser doesn't support playbackRate functionality. - */ - function _checkPlaybackRates() { - var video = document.createElement('video'); - - // If browser supports, 1.0 should be returned by playbackRate - // property. In this case, function return True. Otherwise, False will - // be returned. - return Boolean(video.playbackRate); - } - - // Hide speed control. - function _hideSpeedControl(state) { - state.el.find('div.speeds').hide(); - } - - // Get previous element in array or cyles back to the last if it is the - // first. - function _previousSpeedLink(speedLinks, index) { - return $(speedLinks.eq(index < 1 ? speedLinks.length - 1 : index - 1)); - } - - // Get next element in array or cyles back to the first if it is the last. - function _nextSpeedLink(speedLinks, index) { - return $(speedLinks.eq(index >= speedLinks.length - 1 ? 0 : index + 1)); - } - - function _speedLinksFocused(state) { - var speedLinks = state.videoSpeedControl.videoSpeedsEl - .find('a.speed_link'); - return speedLinks.is(':focus'); - } - - function _openMenu(state) { - // When speed entries have focus, the menu stays open on - // mouseleave. A clickHandler is added to the window - // element to have clicks close the menu when they happen - // outside of it. - $(window).on('click.speedMenu', _clickHandler.bind(state)); - state.videoSpeedControl.el.addClass('open'); - } - - function _closeMenu(state) { - // Remove the previously added clickHandler from window element. - $(window).off('click.speedMenu'); - state.videoSpeedControl.el.removeClass('open'); - } - - // Various event handlers. They all return false to stop propagation and - // prevent default behavior. - function _clickHandler(event) { - var target = $(event.currentTarget); - - this.videoSpeedControl.el.removeClass('open'); - if (target.is('a.speed_link')) { - this.videoSpeedControl.changeVideoSpeed.call(this, event); - } - - return false; - } - - // We do not use _openMenu and _closeMenu in the following two handlers - // because we do not want to add an unnecessary clickHandler to the window - // element. - function _mouseEnterHandler(event) { - this.videoSpeedControl.el.addClass('open'); - - return false; - } - - function _mouseLeaveHandler(event) { - // Only close the menu is no speed entry has focus. - if (!_speedLinksFocused(this)) { - this.videoSpeedControl.el.removeClass('open'); - } -          - return false; - } - - function _keyDownHandler(event) { - var KEY = $.ui.keyCode, - keyCode = event.keyCode, - target = $(event.currentTarget), - speedButtonLink = this.videoSpeedControl.el.children('a'), - speedLinks = this.videoSpeedControl.videoSpeedsEl - .find('a.speed_link'), - index; - - if (target.is('a.speed_link')) { - - index = target.parent().index(); - - switch (keyCode) { - // Scroll up menu, wrapping at the top. Keep menu open. - case KEY.UP: - _previousSpeedLink(speedLinks, index).focus(); - break; - // Scroll down menu, wrapping at the bottom. Keep menu - // open. - case KEY.DOWN: - _nextSpeedLink(speedLinks, index).focus(); - break; - // Close menu. - case KEY.TAB: - _closeMenu(this); - // Set focus to previous menu button in menu bar - // (Play/Pause button) - if (event.shiftKey) { - this.videoControl.playPauseEl.focus(); - } - // Set focus to next menu button in menu bar - // (Volume button) - else { - this.videoVolumeControl.buttonEl.focus(); - } - break; - // Close menu, give focus to speed control and change - // speed. - case KEY.ENTER: - case KEY.SPACE: - _closeMenu(this); - speedButtonLink.focus(); - this.videoSpeedControl.changeVideoSpeed.call(this, event); - break; - // Close menu and give focus to speed control. - case KEY.ESCAPE: - _closeMenu(this); - speedButtonLink.focus(); - break; + return false; } + + this.render(state.speeds, state.speed); + this.bindHandlers(); + + return true; + }, + + /** + * @desc Create any necessary DOM elements, attach them, and set their, + * initial configuration. + * + * @param {array} speeds List of speeds available for the player. + * @param {string|number} currentSpeed Current speed for the player. + * + */ + render: function (speeds, currentSpeed) { + var self = this, + speedsContainer = this.speedsContainer, + reversedSpeeds = speeds.concat().reverse(), + speedsList = $.map(reversedSpeeds, function (speed, index) { + return [ + '
                    2. ', + '', + speed, 'x', + '', + '
                      3. ' + ].join(''); + }); + + speedsContainer.html(speedsList.join('')); + this.speedLinks = new Iterator(speedsContainer.find('.speed-link')); + this.setSpeed(currentSpeed, true, true); + }, + + /** + * @desc Bind any necessary function callbacks to DOM events (click, + * mousemove, etc.). + * + */ + bindHandlers: function () { + var self = this; + + // Attach various events handlers to the speed menu button. + this.el.on({ + 'mouseenter': this.mouseEnterHandler.bind(this), + 'mouseleave': this.mouseLeaveHandler.bind(this), + 'click': this.clickMenuHandler.bind(this), + 'keydown': this.keyDownMenuHandler.bind(this) + }); + + // Attach click and keydown event handlers to the individual speed + // entries. + this.speedsContainer.on({ + click: this.clickLinkHandler.bind(this), + keydown: this.keyDownLinkHandler.bind(this) + }, 'a.speed-link'); + + this.state.el.on({ + 'speed:set': function (event, speed) { + self.setSpeed(speed, true); + }, + 'speed:render': function (event, speeds, currentSpeed) { + self.render(speeds, currentSpeed); + } + }); + }, + + /** + * @desc Check if playbackRate supports by browser. + * If browser supports, 1.0 should be returned by playbackRate + * property. In this case, function return True. Otherwise, False will + * be returned. + * iOS doesn't support speed change. + * + * @param {string} videoType Type of the video player + * + * @returns {boolean} + * true: Browser support playbackRate functionality. + * false: Browser doesn't support playbackRate functionality. + * + */ + isPlaybackRatesSupported: function (state) { + var isHtml5 = state.videoType === 'html5', + isTouch = state.isTouch, + video = document.createElement('video'); + + return !isTouch || (isHtml5 && !Boolean(video.playbackRate)); + }, + + /** + * @desc Opens speed menu. + * + * @param {boolean} bindEvent If set, attaches click event on window. + * + */ + openMenu: function (bindEvent) { + // When speed entries have focus, the menu stays open on + // mouseleave. A clickHandler is added to the window + // element to have clicks close the menu when they happen + // outside of it. + if (bindEvent) { + $(window).on('click.speedMenu', this.clickMenuHandler.bind(this)); + } + + this.el.addClass('is-opened'); + this.speedButton.attr('tabindex', -1); + }, + + /** + * @desc Closes speed menu. + * + * @param {boolean} unBindEvent If set, detaches click event from window. + * + */ + closeMenu: function (unBindEvent) { + // Remove the previously added clickHandler from window element. + if (unBindEvent) { + $(window).off('click.speedMenu'); + } + + this.el.removeClass('is-opened'); + this.speedButton.attr('tabindex', 0); + }, + + /** + * @desc Sets new current speed for the speed control and triggers + * `speedchange` event if needed. + * + * @param {string|number} speed Speed to be set. + * @param {boolean} silent If set, sets new speed without triggering + * `speedchange` event. + * @param {boolean} forceUpdate If set, updates the speed even if it's + * not differs from current speed. + * + */ + setSpeed: function (speed, silent, forceUpdate) { + if (speed !== this.currentSpeed || forceUpdate) { + this.speedsContainer + .find('li') + .removeClass('is-active') + .siblings("li[data-speed='" + speed + "']") + .addClass('is-active'); + + this.speedButton.find('.value').html(speed + 'x'); + this.currentSpeed = speed; + + if (!silent) { + this.el.trigger('speedchange', [speed]); + } + } + }, + + /** + * @desc Click event handler for the menu. + * + * @param {jquery Event} event + * + */ + clickMenuHandler: function (event) { + this.closeMenu(); + return false; - } - else { + }, + + /** + * @desc Click event handler for speed links. + * + * @param {jquery Event} event + * + */ + clickLinkHandler: function (event) { + var speed = $(event.currentTarget).parent().data('speed'); + + this.closeMenu(); + this.setSpeed(this.state.speedToString(speed)); + + return false; + }, + + /** + * @desc Mouseenter event handler for the menu. + * + * @param {jquery Event} event + * + */ + mouseEnterHandler: function (event) { + this.openMenu(); + + return false; + }, + + /** + * @desc Mouseleave event handler for the menu. + * + * @param {jquery Event} event + * + */ + mouseLeaveHandler: function (event) { + // Only close the menu is no speed entry has focus. + if (!this.speedLinks.list.is(':focus')) { + this.closeMenu(); + } +          + return false; + }, + + /** + * @desc Keydown event handler for the menu. + * + * @param {jquery Event} event + * + */ + keyDownMenuHandler: function (event) { + var KEY = $.ui.keyCode, + keyCode = event.keyCode; + switch(keyCode) { // Open menu and focus on last element of list above it. case KEY.ENTER: case KEY.SPACE: case KEY.UP: - _openMenu(this); - speedLinks.last().focus(); + this.openMenu(true); + this.speedLinks.last().focus(); break; // Close menu. case KEY.ESCAPE: - _closeMenu(this); + this.closeMenu(true); break; } // We do not stop propagation and default behavior on a TAB // keypress. return event.keyCode === KEY.TAB; - } -    } +     }, - /** - * @desc Bind any necessary function callbacks to DOM events (click, - * mousemove, etc.). - * - * @type {function} - * @access private - * - * @param {object} state The object containg the state of the video player. - * All other modules, their parameters, public variables, etc. are - * available via this object. - * - * @this {object} The global window object. - * - * @returns {undefined} - */ - function _bindHandlers(state) { - var speedButton = state.videoSpeedControl.el, - videoSpeeds = state.videoSpeedControl.videoSpeedsEl; - - // Attach various events handlers to the speed menu button. - speedButton.on({ - 'mouseenter': _mouseEnterHandler.bind(state), - 'mouseleave': _mouseLeaveHandler.bind(state), - 'click': _clickHandler.bind(state), - 'keydown': _keyDownHandler.bind(state) - }); - - // Attach click and keydown event handlers to the individual speed - // entries. - videoSpeeds.on('click', 'a.speed_link', _clickHandler.bind(state)) - .on('keydown', 'a.speed_link', _keyDownHandler.bind(state)); - } - - // *************************************************************** - // 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(). - // *************************************************************** - - function setSpeed(speed) { - 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'); - } - - function changeVideoSpeed(event) { - var parentEl = $(event.target).parent(); - - event.preventDefault(); - - if (!parentEl.hasClass('active')) { - this.videoSpeedControl.currentSpeed = parentEl.data('speed'); - - this.videoSpeedControl.setSpeed( - // To meet the API expected format. - parseFloat(this.videoSpeedControl.currentSpeed) - .toFixed(2) - .replace(/\.00$/, '.0') - ); - - this.trigger( - 'videoPlayer.onSpeedChange', - this.videoSpeedControl.currentSpeed - ); - } - } - - function reRender(params) { - var _this = this; - - this.videoSpeedControl.videoSpeedsEl.empty(); - this.videoSpeedControl.videoSpeedsEl.find('li').removeClass('active'); - this.videoSpeedControl.speeds = params.newSpeeds; - - $.each(this.videoSpeedControl.speeds, function (index, speed) { - var link, listItem; - - link = '' + speed + 'x'; - - listItem = $('
                    3. ' + link + '
                      4. '); - - if (speed === params.currentSpeed) { - listItem.addClass('active'); + /** + * @desc Keydown event handler for speed links. + * + * @param {jquery Event} event + * + */ + keyDownLinkHandler: function (event) { + // ALT key is used to change (alternate) the function of + // other pressed keys. In this, do nothing. + if (event.altKey) { + return true; } - _this.videoSpeedControl.videoSpeedsEl.prepend(listItem); - }); + var KEY = $.ui.keyCode, + self = this, + parent = $(event.currentTarget).parent(), + index = parent.index(), + speed = parent.data('speed'); - // Re-attach all events with their appropriate callbacks to the - // newly generated elements. - _bindHandlers(this); - } + switch (event.keyCode) { + // Close menu. + case KEY.TAB: + // Closes menu after 25ms delay to change `tabindex` after + // finishing default behavior. + setTimeout(function () { + self.closeMenu(true); + }, 25); + return true; + // Close menu and give focus to speed control. + case KEY.ESCAPE: + this.closeMenu(true); + this.speedButton.focus(); + + return false; + // Scroll up menu, wrapping at the top. Keep menu open. + case KEY.UP: + // Shift + Arrows keyboard shortcut might be used by + // screen readers. In this, do nothing. + if (event.shiftKey) { + return true; + } + + this.speedLinks.prev(index).focus(); + return false; + // Scroll down menu, wrapping at the bottom. Keep menu + // open. + case KEY.DOWN: + // Shift + Arrows keyboard shortcut might be used by + // screen readers. In this, do nothing. + if (event.shiftKey) { + return true; + } + + this.speedLinks.next(index).focus(); + return false; + // Close menu, give focus to speed control and change + // speed. + case KEY.ENTER: + case KEY.SPACE: + this.closeMenu(true); + this.speedButton.focus(); + this.setSpeed(this.state.speedToString(speed)); + + return false; + } + + return true; +     } + }; + + return SpeedControl; }); }(RequireJS.requirejs, RequireJS.require, RequireJS.define)); 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 bfb4fab566..0e70a8a8fc 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 @@ -138,7 +138,7 @@ function (Sjson, AsyncProcess) { onContainerMouseEnter: function (event) { event.preventDefault(); - $(event.currentTarget).addClass('open'); + $(event.currentTarget).addClass('is-opened'); }, /** @@ -149,7 +149,7 @@ function (Sjson, AsyncProcess) { onContainerMouseLeave: function (event) { event.preventDefault(); - $(event.currentTarget).removeClass('open'); + $(event.currentTarget).removeClass('is-opened'); }, /** @@ -364,7 +364,7 @@ function (Sjson, AsyncProcess) { link = $('' + label + ''); if (currentLang === code) { - li.addClass('active'); + li.addClass('is-active'); } li.append(link); @@ -381,9 +381,9 @@ function (Sjson, AsyncProcess) { if (state.lang !== langCode) { state.lang = langCode; state.storage.setItem('language', langCode); - el .addClass('active') + el .addClass('is-active') .siblings('li') - .removeClass('active'); + .removeClass('is-active'); self.fetchCaption(); } diff --git a/common/lib/xmodule/xmodule/video_module/video_module.py b/common/lib/xmodule/xmodule/video_module/video_module.py index f3916810c1..27029dd327 100644 --- a/common/lib/xmodule/xmodule/video_module/video_module.py +++ b/common/lib/xmodule/xmodule/video_module/video_module.py @@ -71,6 +71,7 @@ class VideoModule(VideoFields, VideoStudentViewHandlers, XModule): resource_string(module, 'js/src/video/00_resizer.js'), resource_string(module, 'js/src/video/00_async_process.js'), resource_string(module, 'js/src/video/00_sjson.js'), + resource_string(module, 'js/src/video/00_iterator.js'), resource_string(module, 'js/src/video/01_initialize.js'), resource_string(module, 'js/src/video/025_focus_grabber.js'), resource_string(module, 'js/src/video/02_html5_video.js'), diff --git a/lms/djangoapps/courseware/features/video.py b/lms/djangoapps/courseware/features/video.py index aab009bee7..6e2463453a 100644 --- a/lms/djangoapps/courseware/features/video.py +++ b/lms/djangoapps/courseware/features/video.py @@ -218,14 +218,14 @@ def navigate_to_an_item_in_a_sequence(number): def change_video_speed(speed): - world.browser.execute_script("$('.speeds').addClass('open')") + world.browser.execute_script("$('.speeds').addClass('is-opened')") speed_css = 'li[data-speed="{0}"] a'.format(speed) world.wait_for_visible('.speeds') world.css_click(speed_css) def open_menu(menu): - world.browser.execute_script("$('{selector}').parent().addClass('open')".format( + world.browser.execute_script("$('{selector}').parent().addClass('is-opened')".format( selector=VIDEO_MENUS[menu] )) @@ -379,7 +379,7 @@ def open_video(_step, player_id): @step('video "([^"]*)" should start playing at speed "([^"]*)"$') def check_video_speed(_step, player_id, speed): - speed_css = '.speeds p.active' + speed_css = '.speeds .value' assert world.css_has_text(speed_css, '{0}x'.format(speed)) @@ -397,7 +397,7 @@ def video_is_rendered(_step, mode): } html_tag = modes[mode.lower()] assert world.css_find('.video {0}'.format(html_tag)).first - assert world.is_css_present('.speed_link') + assert world.is_css_present('.speed-link') @step('videos have rendered in "([^"]*)" mode$') @@ -412,7 +412,7 @@ def videos_are_rendered(_step, mode): actual = len(world.css_find('.video {0}'.format(html_tag))) expected = len(world.css_find('.xmodule_VideoModule')) assert actual == expected - assert world.is_css_present('.speed_link') + assert world.is_css_present('.speed-link') @step('all sources are correct$') @@ -494,8 +494,8 @@ def select_language(_step, code): world.wait_for_present('.lang.open') world.css_click(selector) - assert world.css_has_class(selector, 'active') - assert len(world.css_find(VIDEO_MENUS["language"] + ' li.active')) == 1 + assert world.css_has_class(selector, 'is-active') + assert len(world.css_find(VIDEO_MENUS["language"] + ' li.is-active')) == 1 # Make sure that all ajax requests that affects the display of captions are finished. # For example, request to get new translation etc. diff --git a/lms/templates/video.html b/lms/templates/video.html index 49cb3365ff..ea6f3a9cb0 100644 --- a/lms/templates/video.html +++ b/lms/templates/video.html @@ -72,11 +72,11 @@
                      - -

                      ${_('Speed')}

                      -

                      +                       + ${_('Speed')} + - +