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

fix(FEC-7935): fix text selection logic #55

Merged
merged 7 commits into from
Jul 12, 2018
Merged

fix(FEC-7935): fix text selection logic #55

merged 7 commits into from
Jul 12, 2018

Conversation

OrenMe
Copy link
Contributor

@OrenMe OrenMe commented Jul 11, 2018

Description of the Changes

use shaka setTextTrackVisibility to set text track display mode according to useNativeTextTrack config flag.
This change is part of kaltura/playkit-js-hls#65 and kaltura/playkit-js#263 - removing the overhead of handling in playkit-js is possible by using the Shaka APIs to set track visibility.

CheckLists

  • changes have been done against master branch, and PR does not conflict
  • new unit / functional tests have been added (whenever applicable)
  • test are passing in local environment
  • Travis tests are passing (or test results are not worse than on master branch :))
  • Docs have been updated

…ording to `useNativeTextTrack` config flag
@OrenMe OrenMe self-assigned this Jul 11, 2018
dan-ziv
dan-ziv previously approved these changes Jul 11, 2018
@@ -493,6 +495,7 @@ export default class DashAdapter extends BaseMediaSourceAdapter {
*/
selectTextTrack(textTrack: TextTrack): void {
if (this._shaka && (textTrack instanceof TextTrack) && !textTrack.active && (textTrack.kind === 'subtitles' || textTrack.kind === 'captions')) {
this._shaka.setTextTrackVisibility(this._config.textTrackVisibile);
Copy link
Contributor

@yairans yairans Jul 12, 2018

Choose a reason for hiding this comment

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

better to set it on init as before.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we need it here, because for when calling hideTextTracks, e.g. setting them OFF, we call setTextTrackVisibility(false) so we need to always toggle it back.
There's no option in Shaka API to set subtitle OFF, just setting the display mode, so we have to call it when setting text track display of selectTextTrack

yairans
yairans previously approved these changes Jul 12, 2018
@OrenMe OrenMe merged commit cefc2ce into master Jul 12, 2018
@OrenMe OrenMe deleted the FEC-7935 branch July 12, 2018 09:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

3 participants