Skip to content

Persist speed preferences between videos. #2230

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
polesye merged 1 commit into from
Jan 24, 2014
Merged

Persist speed preferences between videos. #2230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes,
in roughly chronological order, most recent first. Add your entries at or near
the top. Include a label indicating the component affected.

Blades: Video player persist speed preferences between videos. BLD-237.

Blades: Change the download video field to a dropdown that will allow students
to download the first source listed in the alternate sources. BLD-364.

Expand Down
4 changes: 3 additions & 1 deletion common/lib/xmodule/xmodule/js/fixtures/video.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
<div
id="video_id"
class="video closed"
data-streams="0.75:7tqY6eQzVhE,1.0:cogebirgzzM"
data-streams="0.5:7tqY6eQzVhE,1.0:cogebirgzzM,1.5:abcdefghijkl"
data-show-captions="true"
data-save-state-url="/save_user_state"
data-speed="1.5"
data-start=""
data-end=""
data-caption-asset-path="/static/subs/"
Expand Down
2 changes: 2 additions & 0 deletions common/lib/xmodule/xmodule/js/fixtures/video_all.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
id="video_id"
class="video closed"
data-show-captions="true"
data-save-state-url="/save_user_state"
data-speed="1.5"
data-start=""
data-end=""
data-caption-asset-path="/static/subs/"
Expand Down
2 changes: 2 additions & 0 deletions common/lib/xmodule/xmodule/js/fixtures/video_html5.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
id="video_id"
class="video closed"
data-show-captions="true"
data-save-state-url="/save_user_state"
data-speed="1.5"
data-start=""
data-end=""
data-caption-asset-path="/static/subs/"
Expand Down
4 changes: 3 additions & 1 deletion common/lib/xmodule/xmodule/js/fixtures/video_no_captions.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
<div
id="video_id"
class="video closed"
data-streams="0.75:7tqY6eQzVhE,1.0:cogebirgzzM"
data-streams="0.5:7tqY6eQzVhE,1.0:cogebirgzzM,1.5:abcdefghijkl"
data-show-captions="false"
data-save-state-url="/save_user_state"
data-speed="1.5"
data-start=""
data-end=""
data-caption-asset-path="/static/subs/"
Expand Down
4 changes: 3 additions & 1 deletion common/lib/xmodule/xmodule/js/fixtures/video_yt_multiple.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
<div
id="video_id1"
class="video closed"
data-streams="0.75:7tqY6eQzVhE,1.0:cogebirgzzM"
data-streams="0.5:7tqY6eQzVhE,1.0:cogebirgzzM,1.5:abcdefghijkl"
data-show-captions="true"
data-save-state-url="/save_user_state"
data-speed="1.5"
data-start=""
data-end=""
data-caption-asset-path="/static/subs/"
Expand Down
6 changes: 6 additions & 0 deletions common/lib/xmodule/xmodule/js/spec/helper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,10 @@
id: 'cogebirgzzM',
duration: 200
},
'abcdefghijkl': {
id: 'abcdefghijkl',
duration: 400
},
bogus: {
duration: 100
}
Expand Down Expand Up @@ -189,6 +193,8 @@
settings.url.match(/.+\/problem_(check|reset|show|save)$/)
) {
// Do nothing.
} else if (settings.url == '/save_user_state') {
return {success: true};
} else {
throw 'External request attempted for ' +
settings.url +
Expand Down
10 changes: 5 additions & 5 deletions common/lib/xmodule/xmodule/js/spec/video/cookie_storage_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ function (CookieStorage) {
it('unload', function () {
var expected = JSON.stringify({
storage: {
'item_2': {
item_2: {
value: 'value_2',
session: false
}
Expand All @@ -51,7 +51,7 @@ function (CookieStorage) {
describe('methods: ', function () {
var data = {
storage: {
'item_1': {
item_1: {
value: 'value_1',
session: false
}
Expand All @@ -69,15 +69,15 @@ function (CookieStorage) {
it('pass correct data', function () {
var expected = JSON.stringify({
storage: {
'item_1': {
item_1: {
value: 'value_1',
session: false
},
'item_2': {
item_2: {
value: 'value_2',
session: false
},
'item_3': {
item_3: {
value: 'value_3',
session: true
},
Expand Down
89 changes: 39 additions & 50 deletions common/lib/xmodule/xmodule/js/spec/video/general_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@

beforeEach(function () {
jasmine.stubRequests();
this.videosDefinition = '0.75:7tqY6eQzVhE,1.0:cogebirgzzM';
this['7tqY6eQzVhE'] = '7tqY6eQzVhE';
this['cogebirgzzM'] = 'cogebirgzzM';
});

afterEach(function () {
Expand All @@ -17,7 +14,7 @@
describe('YT', function () {
beforeEach(function () {
loadFixtures('video.html');
$.cookie.andReturn('0.75');
$.cookie.andReturn('0.50');
});

describe('by default', function () {
Expand All @@ -35,17 +32,18 @@

it('parse the videos', function () {
expect(this.state.videos).toEqual({
'0.75': this['7tqY6eQzVhE'],
'1.0': this['cogebirgzzM']
'0.50': '7tqY6eQzVhE',
'1.0': 'cogebirgzzM',
'1.50': 'abcdefghijkl'
});
});

it('parse available video speeds', function () {
expect(this.state.speeds).toEqual(['0.75', '1.0']);
expect(this.state.speeds).toEqual(['0.50', '1.0', '1.50']);
});

it('set current video speed via cookie', function () {
expect(this.state.speed).toEqual('0.75');
expect(this.state.speed).toEqual('1.50');
});
});
});
Expand Down Expand Up @@ -157,7 +155,7 @@
});

it('set current video speed via cookie', function () {
expect(state.speed).toEqual('0.75');
expect(state.speed).toEqual('1.50');
});
});

Expand Down Expand Up @@ -190,16 +188,18 @@

describe('with speed', function () {
it('return the video id for given speed', function () {
expect(state.youtubeId('0.75'))
.toEqual(this['7tqY6eQzVhE']);
expect(state.youtubeId('0.50'))
.toEqual('7tqY6eQzVhE');
expect(state.youtubeId('1.0'))
.toEqual(this['cogebirgzzM']);
.toEqual('cogebirgzzM');
expect(state.youtubeId('1.50'))
.toEqual('abcdefghijkl');
});
});

describe('without speed', function () {
it('return the video id for current speed', function () {
expect(state.youtubeId()).toEqual(this.cogebirgzzM);
expect(state.youtubeId()).toEqual('abcdefghijkl');
});
});
});
Expand Down Expand Up @@ -314,44 +314,25 @@
});

describe('setSpeed', function () {

describe('YT', function () {
beforeEach(function () {
loadFixtures('video.html');
state = new Video('#example');
});

describe('when new speed is available', function () {
beforeEach(function () {
state.setSpeed('0.75', true);
});

it('set new speed', function () {
expect(state.speed).toEqual('0.75');
});

it('save setting for new speed', function () {
expect($.cookie).toHaveBeenCalledWith(
'video_speed',
'0.75',
{
expires: 3650,
path: '/'
}
);
});
});

describe('when new speed is not available', function () {
beforeEach(function () {
state.setSpeed('1.75');
});
it('check mapping', function () {
var map = {
'0.75': '0.50',
'1.25': '1.50'
};

it('set speed to 1.0x', function () {
expect(state.speed).toEqual('1.0');
$.each(map, function(key, expected) {
state.setSpeed(key, true);
expect(state.speed).toBe(expected);
});
});
});

describe('HTML5', function () {
beforeEach(function () {
loadFixtures('video_html5.html');
Expand All @@ -368,14 +349,9 @@
});

it('save setting for new speed', function () {
expect($.cookie).toHaveBeenCalledWith(
'video_speed',
'0.75',
{
expires: 3650,
path: '/'
}
);

expect(state.storage.getItem('general_speed')).toBe('0.75');
expect(state.storage.getItem('video_speed_' + state.id)).toBe('0.75');
});
});

Expand All @@ -388,6 +364,19 @@
expect(state.speed).toEqual('1.0');
});
});

it('check mapping', function () {
var map = {
'0.25': '0.75',
'0.50': '0.75',
'2.0': '1.50'
};

$.each(map, function(key, expected) {
state.setSpeed(key, true);
expect(state.speed).toBe(expected);
});
});
});
});

Expand All @@ -398,7 +387,7 @@
});

it('return duration for current video', function () {
expect(state.getDuration()).toEqual(200);
expect(state.getDuration()).toEqual(400);

@valera-rozuvan valera-rozuvan Jan 21, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@polesye What influenced the change from 200 to 400?

@polesye polesye Jan 21, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New initial property speed that has value 1.5 by default. See video.html#L10

});
});

Expand Down
12 changes: 6 additions & 6 deletions common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,9 @@

it('create video caption', function () {
expect(state.videoCaption).toBeDefined();
expect(state.youtubeId()).toEqual('Z5KLxerq05Y');
expect(state.speed).toEqual('1.0');
expect(state.config.caption_asset_path)
expect(state.youtubeId('1.0')).toEqual('Z5KLxerq05Y');
expect(state.speed).toEqual('1.50');
expect(state.config.captionAssetPath)
.toEqual('/static/subs/');
});

Expand All @@ -47,7 +47,7 @@
expect(state.videoSpeedControl.el).toHaveClass('speeds');
expect(state.videoSpeedControl.speeds)
.toEqual([ '0.75', '1.0', '1.25', '1.50' ]);
expect(state.speed).toEqual('1.0');
expect(state.speed).toEqual('1.50');
});

it('create video progress slider', function () {
Expand Down Expand Up @@ -395,7 +395,7 @@
'speed_change_video',
{
current_time: state.videoPlayer.currentTime,
old_speed: '1.0',
old_speed: '1.50',
new_speed: '0.75'
}
);
Expand All @@ -406,7 +406,7 @@
});

it('set video speed to the new speed', function () {
expect(state.setSpeed).toHaveBeenCalledWith('0.75', false);
expect(state.setSpeed).toHaveBeenCalledWith('0.75', true);
});
});

Expand Down
2 changes: 1 addition & 1 deletion common/lib/xmodule/xmodule/js/spec/video/video_speed_control_spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
expect(secondaryControls).toContain('.speeds');
expect(secondaryControls).toContain('.video_speeds');
expect(secondaryControls.find('p.active').text())
.toBe('1.0x');
.toBe('1.50x');
expect(li.filter('.active')).toHaveData(
'speed', state.videoSpeedControl.currentSpeed
);
Expand Down
4 changes: 1 addition & 3 deletions common/lib/xmodule/xmodule/js/src/video/00_cookie_storage.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@ function() {
* @param {string} namespace Namespace that is used to store data.
* @return {object} CookieStorage API.
*/


var CookieStorage = function (namespace) {
var Storage;

Expand Down Expand Up @@ -73,7 +71,7 @@ function() {
});

$.cookie(namespace, JSON.stringify(Storage), {
expires: -1,
expires: 3650,

@auraz auraz Jan 22, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 3650?

@polesye polesye Jan 22, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This value is used in video player. It means that, cookie will be stored days (3650) from now.

@auraz auraz Jan 22, 2014

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 years?

@polesye polesye Jan 22, 2014

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, looks so :)

path: '/'
});
};
Expand Down
Loading