Skip to content

Commit

Permalink
fix: chapters and cuepoints interface doesnt reflect internal types (#…
Browse files Browse the repository at this point in the history
…977)

When we released chapters we unified the chapter and cuepoint types
under the hood, making both support an optional `endTime` with cuepoints
still supporting the legacy shape using `time` instead of `startTime`.
Our `addChapters` and `addCuePoints` methods on the player don't use
these new types though. `endTime` is currently required by the
`addChapters` method, but shouldn't be, and only the legacy type is
required by `addCuePoints`.

These changes re-use our existing internal types for `Chapter` and
`CuePoint` for the external interface.

Fixes #947
  • Loading branch information
AdamJaggard authored Aug 21, 2024
1 parent 2722b6e commit e3eadec
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ function MuxPlayerPage() {
const [activeChapter, setActiveChapter] = useState<Chapter>(undefined);

function addChaptersToPlayer(playerEl: MuxPlayerElement) {
playerEl.addChapters(chapters);
playerEl.addChapters(chapters);
}

return (
Expand Down
6 changes: 4 additions & 2 deletions packages/mux-player/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import type {
MaxResolutionValue,
MinResolutionValue,
RenditionOrderValue,
Chapter,
CuePoint,
} from '@mux/playback-core';
import VideoApiElement from './video-api';
import {
Expand Down Expand Up @@ -1552,7 +1554,7 @@ class MuxPlayerElement extends VideoApiElement implements MuxPlayerElement {
this.media._hlsConfig = val;
}

async addCuePoints<T = any>(cuePoints: { time: number; value: T }[]) {
async addCuePoints<T = any>(cuePoints: CuePoint<T>[]) {
this.#init();

// NOTE: This condition should never be met. If it is, there is a bug (CJP)
Expand All @@ -1571,7 +1573,7 @@ class MuxPlayerElement extends VideoApiElement implements MuxPlayerElement {
return this.media?.cuePoints ?? [];
}

addChapters(chapters: { startTime: number; endTime: number; value: string }[]) {
addChapters(chapters: Chapter[]) {
this.#init();

// NOTE: This condition should never be met. If it is, there is a bug (CJP)
Expand Down
6 changes: 4 additions & 2 deletions packages/mux-video/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ import type {
MaxResolutionValue,
MinResolutionValue,
RenditionOrderValue,
Chapter,
CuePoint,
} from '@mux/playback-core';
import { getPlayerVersion } from './env';
// this must be imported after playback-core for the polyfill to be included
Expand Down Expand Up @@ -515,7 +517,7 @@ class MuxVideoBaseElement extends CustomVideoElement implements Partial<MuxMedia
return getSeekable(this.nativeEl);
}

async addCuePoints<T = any>(cuePoints: { time: number; value: T }[]) {
async addCuePoints<T = any>(cuePoints: CuePoint<T>[]) {
return addCuePoints(this.nativeEl, cuePoints);
}

Expand All @@ -527,7 +529,7 @@ class MuxVideoBaseElement extends CustomVideoElement implements Partial<MuxMedia
return getCuePoints(this.nativeEl);
}

async addChapters(chapters: { startTime: number; endTime: number; value: string }[]) {
async addChapters(chapters: Chapter[]) {
return addChapters(this.nativeEl, chapters);
}

Expand Down

0 comments on commit e3eadec

Please sign in to comment.