Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
README.md
Outdated
| } | ||
| ``` | ||
|
|
||
| Kind can be any of the following: "main", "translation", "descriptions", or "commentary". |
There was a problem hiding this comment.
Should we also specify what the provenance values could be?
There was a problem hiding this comment.
Updated to include those values as well.
types/player.d.ts
Outdated
| * @throws {InvalidParameterError} If no track was available with the specified language | ||
| * @throws {InvalidTrackError} If no track was available with the specified language and kind | ||
| */ | ||
| enableAudioTrack(language: string, kind?: string): Promise<VimeoAudioTrack>; |
There was a problem hiding this comment.
Should we provide specific types for kind and VimeoAudioTrack?
There was a problem hiding this comment.
Good call. Added types for AudioKind, AudioProvenance, AudioLanguage (just a string, but includes more descriptions for ISO codes), and VimeoAudioTrack that includes all of the above.
types/player.d.ts
Outdated
| * @throws {InvalidParameterError} If no track was available with the specified language | ||
| * @throws {InvalidTrackError} If no track was available with the specified language and kind | ||
| */ | ||
| enableAudioTrack(language: string, kind?: string): Promise<VimeoAudioTrack>; |
There was a problem hiding this comment.
Can we document language and kind more thoroughly? We should give more examples of what they can be and where you can find the available options in the player or Vimeo.com.
There was a problem hiding this comment.
I added AudioLanguage and AudioKind types that include more descriptions/links for both of them, and a union for the kinds to specify the options.
src/player.js
Outdated
| /** | ||
| * A representation of an audio track on a video. | ||
| * | ||
| * @typedef {Object} VimeoAudioTrack |
There was a problem hiding this comment.
can we import the typescript type for this, instead of defining it twice?
README.md
Outdated
| - [getTextTracks(): Promise<object[], Error>](#gettexttracks-promiseobject-error) | ||
| - [getAudioTracks(): Promise<object[], Error>](#getaudiotracks-promiseobject-error) | ||
| - [getEnabledAudioTrack(): Promise<object[], Error>](#getenabledaudiotrack-promiseobject-error) | ||
| - [getMainAudioTrack(): Promise<object[], Error>](#getmainaudiotrack-promiseobject-error) |
There was a problem hiding this comment.
do we still want to call this Main if we're referring to it as default in the setter?
There was a problem hiding this comment.
Good catch. Updated, and will make the change internally
types/player.d.ts
Outdated
|
|
||
| getAudioTracks(): Promise<VimeoAudioTrack[]>; | ||
| getEnabledAudioTrack(): Promise<VimeoAudioTrack | undefined>; | ||
| getMainAudioTrack(): Promise<VimeoAudioTrack | undefined>; |
There was a problem hiding this comment.
do we still want to call this Main if we're referring to it as default in the setter?
src/player.js
Outdated
| * @param {string} language The two‐letter language code. | ||
| * @param {string} [kind] The kind of track to enable (main, translation, descriptions, commentary). |
There was a problem hiding this comment.
I think we could also import the aliases (AudioLanguage and AudioKind) for these, right?
3 participants
This PR adds support for audio track methods on the player SDK.