Imagekit video player #1
Conversation
…teTextTrack options
…dded github pages configurations.
…ss video player modules
…efinitions for improved maintainability
| private ikGlobalSettings_: IKPlayerOptions; | ||
| private currentSource_: SourceOptions | SourceOptions[] | null = null; | ||
| private originalCurrentSource_: SourceOptions | SourceOptions[] | null = null; | ||
| private playlistManger_?: PlaylistManager; |
| if (!src.recommendations) return; | ||
|
|
||
| const overlay = this.player.getChild('RecommendationsOverlay'); | ||
| console.log('RecommendationsOverlay:', overlay); |
There was a problem hiding this comment.
remove console log before releasing
| // if poster is already prepared, use it directly | ||
| // @ts-ignore | ||
| if (currentSource_?.prepared?.poster) { | ||
| console.log("Using prepared poster src") |
There was a problem hiding this comment.
remove console log before releasing
| try { | ||
| const res = await fetch(src.chapters.url); | ||
| if (!res.ok) { | ||
| this.player.log.warn(`VTT fetch failed with status: (${res.status}); skipping chapters.`); |
There was a problem hiding this comment.
return statement after this warning is missing
| src.prepared = {} | ||
| } | ||
| // @ts-ignore | ||
| src.prepared.src = prepared[0].src; |
| this.unloadPlaylist(); | ||
|
|
||
| this.playlist_ = playlist; | ||
| this.autoAdvance_ = new AutoAdvance(this.player_, this.playNext_); |
There was a problem hiding this comment.
we should dispose autoadvance, by doing this.autoAdvance_?.fullReset() before creating new one
| if (!currentTime || !duration || !isFinite(duration) || !isFinite(currentTime)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
what happens if currentTime or duration === 0? duration i understand won't cause issues but currentTime can be 0
| private setupPresentUpcoming_() { | ||
| // Always remove the old component first | ||
| this.presentUpcomingComponent_?.dispose(); | ||
| this.player_.off('timeupdate', this.handleTimeUpdateForUpcoming_); | ||
|
|
||
| if (this.presentUpcomingThreshold_ === null) { | ||
| // If the feature is disabled, we're done. | ||
| return; | ||
| } | ||
|
|
||
| // Create and add the new component to the player | ||
| this.presentUpcomingComponent_ = this.player_.addChild('PresentUpcoming', this.playerOptions_) as PresentUpcoming; | ||
|
|
||
| this.presentUpcomingComponent_.on('dismiss', this.handleUpcomingDismiss_); | ||
|
|
||
|
|
||
| // Listen for time updates to know when to show it | ||
| this.player_.on('timeupdate', this.handleTimeUpdateForUpcoming_); | ||
|
|
||
| // Also hide it immediately when a new source starts loading | ||
| this.player_.on('loadstart', () => { | ||
| this.presentUpcomingComponent_?.hide(); | ||
| // --- CHANGE: MOVED TO HERE --- | ||
| // This is the correct place to reset the dismissed state for the upcoming video. | ||
| this.isUpcomingDismissed_ = false; | ||
| }); | ||
| } |
There was a problem hiding this comment.
we should have a function for loadstart body and remove the listener like we do for timeupdate
| top: 50%; | ||
| left: 50%; | ||
| transform: translate(-50%, -50%); | ||
| animation: pulse 2s infinite; |
| .vjs-shoppable-toggle .icon-open { | ||
| opacity: 1; | ||
| } | ||
|
|
||
| .vjs-shoppable-toggle .icon-close { | ||
| opacity: 1; | ||
| } |
There was a problem hiding this comment.
open and close both have opacity 1
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 785dd49517
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| inputs.forEach((src) => { | ||
| if (typeof src === 'object' && src !== null && !this.hasPreparedSrc(src)) { | ||
| ensurePrepared(src as AugmentedSourceOptions).src = prepared[0].src; | ||
| } |
There was a problem hiding this comment.
When multiple sources are passed (e.g., different qualities or HLS+MP4), this loop writes prepared[0].src into every input’s prepared.src. If the caller reuses the same source objects later, hasPreparedSrc will short‑circuit prepareSource and all entries will resolve to the first URL, breaking source selection and potentially serving the wrong file. Each input should get its corresponding prepared[i].src so cached preparation stays aligned with the source array.
Useful? React with 👍 / 👎.
| .finally(() => { | ||
| // hide the spinner and show the big play button | ||
| bigPlay && bigPlay.show(); | ||
| this.player.removeClass('vjs-waiting'); | ||
| spinner?.el()?.setAttribute('aria-hidden', 'true'); |
There was a problem hiding this comment.
The finally block always hides the spinner and shows the big play button, even if a newer src() call is still in flight. If a user switches sources quickly or an earlier request resolves after a later one starts, the older promise will flip the UI to “not loading” while the latest source is still preparing, causing incorrect loading state. Consider checking myCallId === this.srcCallVersion before toggling UI state here.
Useful? React with 👍 / 👎.
No description provided.