feat: setting which tracks should be considered for seek and autopause#890
feat: setting which tracks should be considered for seek and autopause#890rodrigo-suarezmajor wants to merge 1 commit intokillergerbah:mainfrom
Conversation
|
Thanks for this, will review once 1.14 is out |
killergerbah
left a comment
killergerbah
left a comment
There was a problem hiding this comment.
Really appreciate the PR, fixes a pretty bad issue. Left some minor comments.
| playbackRate = 2, | ||
| } | ||
|
|
||
| export type SeekableTracks = [boolean, boolean, boolean]; // [firstTrack, secondTrack, thirdTrack] |
There was a problem hiding this comment.
Any reason why you picked a list of booleans vs a list of numbers? At first thought a list of numbers would generally be more compact, easier to understand and extensible, even if we never extend the list of configurable tracks beyond 3.
| } | ||
|
|
||
| subtitlesAt(timestamp: number): SubtitleSlice<T> { | ||
| subtitlesAt(timestamp: number, seekableTracks?: SeekableTracks): SubtitleSlice<T> { |
There was a problem hiding this comment.
I can see why seekableTracks is a convenient parameter but from the perspective of just this class it's not completely obvious why seekable should have the same meaning as showing.
Instead of having this parameter I suggest keeping the track seekability logic outside of this class. If it feels like you're repeating a lot of code then it can be extracted into a function.
| refreshCurrentSubtitle: boolean; | ||
| _preCacheDom; | ||
| dictionaryTrackSettings?: DictionaryTrack[]; | ||
| seekableTracks: () => SeekableTracks; |
There was a problem hiding this comment.
All the other subtitle settings are exposed as public fields, curious why you went with a function just for this one?
2 participants
Added a setting for deciding which subtitles track should be "seekable" through keyboard shortcuts as well as auto-paused on, as mentionend in #742 #385 #402
Default is to consider only the first track, which imo should be the expected behavior. I didn't manage to implement it in the other playback modes easily, there the previous behavior remains.