This repository was archived by the owner on Feb 22, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
[video_player] VTT Support #2878
[video_player] VTT Support #2878
Changes from all commits
9788b63
a66f5d7
6481331
8f8b095
da1c6e4
1a21621
0d3549e
b018ea9
b92f305
1573670
1e53fc5
9034263
d9647a9
2ebf99f
43abfd6
a44edea
d92e820
10e717c
98e21c1
e146366
c05a6f1
50044cc
2aacc0f
928bfb4
ee9e13a
bf3e958
91bba0f
459b7f8
b43b1a8
db547b6
7864399
7afa2c2
a8a5994
0fdc514
1ca27a7
39ba0ae
7391c73
00efa43
8e1dcad
9fdcb05
f0e286b
1350f8e
c72a068
226b9c6
4554771
0b7dde4
7aa17ba
2f686c2
5d09170
78098ca
46aecd8
cacf4dc
5524931
110c115
808b1a0
a041f54
2a16e2e
acf7417
de3c973
495b37c
fc0d049
9db4ec1
c5bbba6
7bb0d53
d47ea7f
4c66c38
db79449
fa0d6dc
a4fcbf7
61a22a5
14468e5
0d68872
c97876c
6c457a5
5ffff31
c8f7c2c
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once the above is removed, this can all condense to:
It would be good to add a safety check that the length is either 2 or 3 and returns null first, as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I'm reading this again: it's weird that _parseWebVTTTimestamp is where captions would be handled. I think the caller—which is already doing a regex match for timestamps anyway—should just use match groups (the regex would need to be adjusted slightly since right now it's making groups for all the sub-pieces of the timestamp, which isn't actually used) to extract just the timestamps, and then pass those here, removing the need for the
milisecondsStyles = dotSections[1].split(" ")
. Then the caption-handling TODO would go in the calling function, not here.But I'm fine with that being something that's cleaned up if/when this functionality is added since I don't want to keep piling changes onto a PR you adopted in the first place :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, I'll put a note in the code and link this comment.