Skip to content
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

Jump to bottom

Improve localisation #1873

Closed
wants to merge 12 commits into from
Closed

Improve localisation #1873

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
679598a
Localisation ignores case and falls back to primary codes
mister-ben Feb 17, 2015
c0617a9
improve tests
mister-ben Feb 18, 2015
0e63261
Merge pull request #2145 from cvrebert/patch-3
mmcc May 13, 2015
6128305
v5.0.0-14
mmcc May 13, 2015
2fc8968
@heff added support for fluid widths, aspect ratios, and metadata def…
heff May 14, 2015
349ff89
v5.0.0-15
heff May 14, 2015
2bf0be7
build: bower.json: remove moot `version` field
cvrebert May 13, 2015
77fc23e
Localisation ignores case and falls back to primary codes
mister-ben Feb 17, 2015
0208712
Cleanup after rebase
mister-ben May 12, 2015
f8b0fb1
Normalise to lowercase when adding languages
mister-ben May 17, 2015
11303fc
Merge branch 'feature/l10n'
mister-ben May 18, 2015
34c1a7b
Merge branch
mister-ben May 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
19 changes: 16 additions & 3 deletions src/js/component.js
View file
Open in desktop
Expand Up @@ -201,12 +201,25 @@ vjs.Component.prototype.createEl = function(tagName, attributes){
return vjs.createEl(tagName, attributes);
};

/**
* Return a localised string if available
*
* @param {String} string String to be localised
* @return {String}
*/
vjs.Component.prototype.localize = function(string){
var lang = this.player_.language(),
languages = this.player_.languages();
if (languages && languages[lang] && languages[lang][string]) {
var lang = ('' + this.player_.language()).toLowerCase();
var languages = this.player_.languages();
if (!languages) {
return string;
}
if (languages[lang] && languages[lang][string]) {
return languages[lang][string];
}
var primaryCode = lang.split('-')[0];
Copy link
Member

Choose a reason for hiding this comment

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

Though I think no one likes it at this point, most of the rest of the code declares variables at the top of the scope.

if (primaryCode !== lang && languages[primaryCode] && languages[primaryCode][string]) {
return languages[primaryCode][string];
}
return string;
};

Expand Down
1 change: 1 addition & 0 deletions src/js/core.js
View file
Open in desktop
Expand Up @@ -149,6 +149,7 @@ if (vjs.CDN_VERSION !== 'GENERATED'+'_CDN_VSN') {
* @return {Object} The resulting global languages dictionary object
*/
vjs.addLanguage = function(code, data){
code = ('' + code).toLowerCase();
if(vjs.options['languages'][code] !== undefined) {
vjs.options['languages'][code] = vjs.util.mergeOptions(vjs.options['languages'][code], data);
} else {
Expand Down
7 changes: 6 additions & 1 deletion src/js/player.js
View file
Open in desktop
Expand Up @@ -49,7 +49,12 @@ vjs.Player = vjs.Component.extend({
this.language_ = options['language'] || vjs.options['language'];

// Update Supported Languages
this.languages_ = options['languages'] || vjs.options['languages'];
var languages = vjs.obj.copy(options['languages'] || vjs.options['languages']);
vjs.options['languages'] = {};
Copy link
Member

Choose a reason for hiding this comment

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

Are you taking the player level language definitions and copying them out to the window.videojs level here? I'm not following. Maybe a comment above would help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrong solution to the problem. Or solution to the wrong problem. I need to rework that.

I've now realised that although this.languages_ is either options['languages'] or vjs.options['languages'];, addLanguage() always adds to vjs.options['languages'];. So a plugin which adds a language might not be able to add it to the player using it, but would add it to others on the page: http://jsbin.com/nomune/1/edit?html,console

Copy link
Member

Choose a reason for hiding this comment

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

Ah, ok. There are two levels of options in video.js: page-level and player-level. I believe this code was originally intending to use player-level language settings if provided and fall back to the page-level language settings otherwise.

vjs.obj.each(languages, function(name, value) {
vjs.addLanguage(name, languages[name]);
});
this.languages_ = vjs.options['languages'];

// Cache for video property values.
this.cache_ = {};
Expand Down
24 changes: 24 additions & 0 deletions test/unit/component.js
View file
Open in desktop
Expand Up @@ -590,3 +590,27 @@ test('should provide interval methods that automatically get cleared on componen

ok(intervalsFired === 5, 'Interval was cleared when component was disposed');
});

test('should ignore case in language codes and try primary code', function() {
expect(3);

var player = PlayerTest.makePlayer({
'languages': {
'en-gb': {
'Good': 'Brilliant'
},
'EN': {
'Good': 'Awesome',
'Error': 'Problem'
}
}
});

var comp = new vjs.Component(player);

player.language('en-gb');
ok(comp.localize('Good') === 'Brilliant', 'Used subcode specific localization');
Copy link
Member

Choose a reason for hiding this comment

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

You can use strictEqual() (for === equality) or equal() (for ==) instead of ok() to get better error messages when these assertions fail.

ok(comp.localize('Error') === 'Problem', 'Used primary code localization');
player.language('en-GB');
ok(comp.localize('Good') === 'Brilliant', 'Ignored case');
});