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.

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

Update ru.json #3654

Closed
wants to merge 5 commits into from
Closed

Update ru.json #3654

wants to merge 5 commits into from

Conversation

Globulopolis
Copy link

Update translation.

"The media could not be loaded, either because the server or network failed or because the format is not supported.": "Невозможно загрузить видео из-за сетевого или серверного сбоя либо формат не поддерживается.",
"The media playback was aborted due to a corruption problem or because the media used features your browser did not support.": "Воспроизведение видео было приостановлено из-за повреждения либо в связи с тем, что видео использует функции, неподдерживаемые вашим браузером.",
"No compatible source was found for this media.": "Совместимые источники для этого видео отсутствуют."
"Play": "Воспроизвести",
Copy link
Member

Choose a reason for hiding this comment

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

Can the indentation be fixed here so the diff only shows the new items that were added?

@gkatsev
Copy link
Member

gkatsev commented Sep 29, 2016

Awesome thanks. This LGTM.
Though, one question, while we have you here, would you be able to help out with the rest of the missing russian translations? If not, that's fine too.

@Globulopolis
Copy link
Author

@gkatsev yes, I can help. I just need to know where the untranslated string used in videojs, like modal dialog. Demo would be awesome.

@gkatsev
Copy link
Member

gkatsev commented Sep 29, 2016

Sure, I can help. Currently busy at the moment but I'll get back to you in a bit.

Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

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

Added some comments about the missing items.
The only one that wasn't easily comment-able is , selected which is also related to captions.
If you were to enable captions, the item that is selected should show that text when you hover over it.

@Globulopolis if those makes sense, would love it if you can update this PR with those.
Thanks!

| | Descriptions |
| | descriptions off |
| | Audio Track |
| ru.json (missing 6 ) | Close Modal Dialog |
Copy link
Member

Choose a reason for hiding this comment

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

"Close Modal Dialog" is shows up as text for the close button on the modal dialog.
You can see this by opening a modal dialog and hovering over the close button:

player.createModal('foo');

| | Descriptions |
| | descriptions off |
| | Audio Track |
| ru.json (missing 6 ) | Close Modal Dialog |
| | The media is encrypted and we do not have the keys to decrypt it. |
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 see an example of this by running the following snippet:

player.error({code: 5});

It'll open an error dialog with this text in it.
To close the dialog, you call error() with null:

player.error(null);
```.

| | Close |
| | Modal Window |
| | This is a modal window |
| | This modal can be closed by pressing the Escape key or activating the close button. |
| | , opens captions settings dialog |
Copy link
Member

Choose a reason for hiding this comment

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

This shows up as title text when you hover to the "caption settings" menu item. To see this, you'd want to enable emulated text tracks in videojs and add captions:

var player = videojs('vid', {
  html5: {
    nativeTextTracks: false
  }
});

Then after playing, if you open the captions menu and hover over the settings item, you can see this text.

| | Close |
| | Modal Window |
| | This is a modal window |
| | This modal can be closed by pressing the Escape key or activating the close button. |
| | , opens captions settings dialog |
| | , opens subtitles settings dialog |
Copy link
Member

Choose a reason for hiding this comment

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

Same as the caption settings dialog, though, I'm not sure we're currently showing it in any case right now but it would be equivalent to the captions setting dialog when we would do it.

| | Close |
| | Modal Window |
| | This is a modal window |
| | This modal can be closed by pressing the Escape key or activating the close button. |
| | , opens captions settings dialog |
| | , opens subtitles settings dialog |
| | , opens descriptions settings dialog |
Copy link
Member

Choose a reason for hiding this comment

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

Same as the caption settings dialog, though, I'm not sure we're currently showing it in any case right now but it would be equivalent to the captions setting dialog when we would do it.

Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

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

Awesome, thanks for the updates!
One small issue.

"Play Video": "Воспроизвести видео",
"Close": "Закрыть",
"Modal Window": "Модальное окно",
"This is a modal window.": "Это модальное окно.",
Copy link
Member

Choose a reason for hiding this comment

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

I think this shouldn't have a period at the end.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, see #3654 (comment)

@Globulopolis
Copy link
Author

@gkatsev all string are translated now. But I found one error.
String https://github.com/videojs/video.js/blob/master/lang/en.json#L34 w/o dot at the end of string, but in https://github.com/videojs/video.js/blob/master/src/js/modal-dialog.js#L135 with a dot at the end.

@gkatsev
Copy link
Member

gkatsev commented Oct 7, 2016

@Globulopolis oh, interesting. I didn't realize that. It seems like other translations already assume it's the phrase without the dot, so, might be just easier to change the modial-dialog to assume it's without a dot :)

@gkatsev
Copy link
Member

gkatsev commented Oct 7, 2016

What do you think?

@Globulopolis
Copy link
Author

Yes, update modal-dialog.js much easier.

@gkatsev
Copy link
Member

gkatsev commented Oct 7, 2016

@Globulopolis could you update it as well? Thanks!

Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

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

Thanks @Globulopolis!

@gkatsev
Copy link
Member

gkatsev commented Oct 7, 2016

Would you be able to split uk.json into a separate PR?

@Globulopolis
Copy link
Author

@gkatsev I don't know how :(

@gkatsev
Copy link
Member

gkatsev commented Oct 7, 2016

heh, I was going to do a more complicated set of instructions but I think this will work more easily:

You could try something like the following:

$ git checkout -b update-uk.json # create a new branch for uk update
$ git push -u origin update-uk.json # push the branch up, can then be made into a PR
$ git checkout master # go back to master branch
$ git reset --hard HEAD~1 # get rid of the top commit in the branch
$ git push -f origin master # override what's currently on github and in this PR

@misteroneill misteroneill added the i18n Relation to localisation and translation. label Oct 17, 2016
gkatsev added a commit that referenced this pull request Nov 3, 2016
@gkatsev
Copy link
Member

gkatsev commented Nov 3, 2016

Fixed via d11fd50

@gkatsev gkatsev closed this Nov 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n Relation to localisation and translation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants