Added proper subtitle switching

SunnyLi / videojs-ass

Advanced SubStation Alpha subtitle plugin for videojs

https://sunnyli.github.io/videojs-ass/example.html

Other

91 stars 22 forks source link

Added proper subtitle switching #19

Closed DoomTay closed 7 years ago

DoomTay commented 7 years ago

This would add "true" subtitle switching, as at the moment, there is only a function for adding subtitle tracks.

This would replace the custom ASS button, and switching would be done through the "built in" caption switcher using "dummy" tracks

Some additional thoughts

This maybe considered a "breaking" change, as it would add to loadNewSubtitle's signature
In my tests, I've found that initializing the plugin, then loading another subtitle track a few seconds later without switching to it immediately will result in both "off" and the first track being highlighted in the captions menu, and switching to the new track will result in both tracks being visible on screen. Setting it to switch immediately works fine.

SunnyLi commented 7 years ago

Good stuff.

I'll test it out ~~in the weekend~~ some time next week and get it merged.

As for the breaking change, we have this in SemVer 2.0:

Major version zero (0.y.z) is for initial development. Anything may change at any time. The public API should not be considered stable.

Oh nvm, https://docs.npmjs.com/misc/semver#caret-ranges-123-025-004

Many authors treat a 0.x version as if the x were the major "breaking-change" indicator.

DoomTay commented 7 years ago

After some tinkering, I'm starting to think the "multiple buttons highlighted" bug may actually be on videojs. If I add a native text track, then a few seconds later add another, all three caption options are "selected" until the next line comes up.

SunnyLi commented 7 years ago

Yay, finally merged!

I've formatted your code a bit and think I've fixed the multiple tracks being highlighted when not using switchImmediately issue.