feat: add full variant to AudioPlayer with waveform and progress bar #35
Conversation
Deploying ui with
|
| Latest commit: |
f8537d6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ab6d0640.ui-6d0.pages.dev |
| Branch Preview URL: | https://new-variant-audioplayer.ui-6d0.pages.dev |
|
@rajkumargaramie looks like there are some conflicts here. Can you fix them? |
There was a problem hiding this comment.
Pull request overview
Adds a new full visual variant to the AudioPlayer component, combining a WaveSurfer waveform with a scrub-able progress bar, and exposes waveform-related exports for external use.
Changes:
- Added
fullvariant styling and behavior toAudioPlayer(WaveSurfer + progress bar). - Refactored
Waveforminto aforwardRefcomponent with an exported imperativeWaveformHandle(for syncing seeks). - Updated Storybook to include the
fullvariant stories and variant selector option.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/components/AudioPlayer/index.ts | Re-exports Waveform and WaveformHandle from AudioPlayer. |
| src/components/AudioPlayer/AudioPlayer.tsx | Implements full variant and adds imperative waveform seeking support via ref. |
| src/components/AudioPlayer/AudioPlayer.stories.tsx | Adds Storybook coverage and controls for the new full variant. |
Comments suppressed due to low confidence (1)
src/components/AudioPlayer/AudioPlayer.tsx:803
onReady={handleWaveformReady}currently callssetState('idle')unconditionally. If the user clicks Play before WaveSurfer finishes loading, the state can flip toplayingand then be reset back toidleon ready, cancelling the play request and desyncing the play button from actual playback. Consider tracking waveform readiness/loading and only transitioning toidlefromloading, or preserving the prior state (e.g., keepplayingif the user already requested playback) and useupdateStatesoonStateChangestays consistent.
<Waveform
ref={waveformRef}
src={src}
isPlaying={isPlaying}
playbackRate={playbackRate}
onReady={handleWaveformReady}
onTimeUpdate={handleWaveformTimeUpdate}
onFinish={handleWaveformFinish}
onSeek={handleWaveformSeek}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| wavesurferRef.current.on('seeking', () => { | ||
| onTimeUpdate(wavesurferRef.current.getCurrentTime()); | ||
| const time = wavesurferRef.current.getCurrentTime(); | ||
| onTimeUpdate(time); | ||
| onSeek(time); | ||
| }); | ||
|
|
||
| wavesurferRef.current.on('interaction', () => { | ||
| onSeek(wavesurferRef.current.getCurrentTime()); | ||
| // Handle seek events (fired after seeking is complete) | ||
| wavesurferRef.current.on('seek', () => { | ||
| const time = wavesurferRef.current.getCurrentTime(); | ||
| onTimeUpdate(time); | ||
| onSeek(time); | ||
| }); |
There was a problem hiding this comment.
WaveSurfer event handlers call both onTimeUpdate and onSeek for both seeking and seek events. This can invoke onTimeUpdate/onSeek multiple times per user action (and continuously while scrubbing), which is redundant and can cause avoidable re-renders/work for consumers. Consider firing onTimeUpdate only from audioprocess/seeking, and onSeek only once when the seek is finalized (pick a single event), or add de-duping so one interaction doesn't emit duplicate updates.
| onReady, | ||
| onTimeUpdate, | ||
| onFinish, | ||
| onSeek, | ||
| waveColor, | ||
| progressColor, |
There was a problem hiding this comment.
The Waveform prop destructuring indentation appears broken (e.g., onReady, onTimeUpdate, etc. are misaligned). This makes the component harder to read and likely indicates the file wasn't run through the formatter after the refactor. Re-run Prettier/formatting to normalize indentation.
| onReady, | |
| onTimeUpdate, | |
| onFinish, | |
| onSeek, | |
| waveColor, | |
| progressColor, | |
| onReady, | |
| onTimeUpdate, | |
| onFinish, | |
| onSeek, | |
| waveColor, | |
| progressColor, |
|
I am working on using Waveform variant with cursor instead of this full variant. So this PR is differed for now. |
2 participants
Uh oh!
There was an error while loading. Please reload this page.