fix: align voice memo waveform highlight with playback position#5917
fix: align voice memo waveform highlight with playback position#5917janicduplessis wants to merge 6 commits into
Conversation
505b28d to
00133ff
Compare
00133ff to
04f43ba
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d9c9c2f5c
ℹ️ 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".
| height={Math.max(5, layout.height * (heightRatio ?? 0))} | ||
| r={40} | ||
| color={ | ||
| heightRatio == null | ||
| ? candleInactiveColor | ||
| : index < scaledCandlePlaybackPosition | ||
| ? candleActiveColor | ||
| index < scaledCandlePlaybackPosition | ||
| ? candleActiveColor |
There was a problem hiding this comment.
Mirror the waveform playback fix on web
When this component renders on web, React Native resolves Waveform.web.tsx instead of this file; that implementation still leaves null padding candles inactive and only rescales when values.length > maxVisibleCandleCount. As a result, web voice memos with padded previews (for example 50 samples in a 51-candle container, or the 15-bar dummy waveform) still do not fill the trailing candles at the end of playback, even though the native path now does. Apply the same scaling/color ordering there if this playback alignment fix is intended to cover voice memos across platforms.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — applied the same changes to Waveform.web.tsx in 4f651b1: the playhead rescales over the drawn candle count whenever it differs from the value count (padded case included, not just simplified), and the playhead check now wins over the filler check in the SVG fill. Mirrors the native logic that was validated by pixel-classifying the rendered candles at the end of playback.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f651b176f
ℹ️ 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".
| } | ||
| return clamp( | ||
| Math.floor((progress.currentTime / progress.duration) * candleCount), | ||
| Math.round((progress.currentTime / progress.duration) * candleCount), |
There was a problem hiding this comment.
Preserve fractional playback before scaling padded waveforms
For voice memos whose preview has fewer samples than the rendered candle count, this rounds the progress against the short candleCount before Waveform rescales it across the padded candles. That makes candlePlaybackPosition become candleCount as soon as playback reaches (candleCount - 0.5) / candleCount, after which the Waveform scaling lights every padded candle even though audio is still playing; for the 15-bar dummy waveform, a 60s memo appears fully highlighted from about 58s onward. Passing a fractional position through to Waveform or rounding only after the final drawn count is known would keep the highlight aligned until actual completion.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right — the value-domain rounding before the Waveform's rescale double-quantized the position, so short previews (like the 15-bar dummy) hit "full" up to half a value-candle early. Fixed in fb4ac8e: candlePlaybackPosition is now passed through as a fractional position and the only quantization left is the index < scaled comparison against the candles actually drawn, on both the native and web waveforms.
There was a problem hiding this comment.
Pull request overview
This PR fixes misalignment between the voice memo waveform’s highlighted “played” region and the actual playback position by keeping the playhead as a fraction until the waveform scales it against the candles actually rendered (including padded filler candles), allowing the highlight to reach the final candle.
Changes:
- Pass a fractional playhead position through
VoiceMemoBlock(no earlyfloor/candleCount - 1clamp), enabling the last candle to highlight. - Rescale playhead position in
Waveform/Waveform.webwhen the rendered candle count differs from the input value count (including padded filler), and allow filler candles to highlight after the playhead passes them. - Adjust candle fill precedence so the playhead-highlight check wins over “filler candle” coloring.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/app/ui/components/PostContent/BlockRenderer.tsx | Computes playhead position as a fraction and clamps to candleCount so the final candle can highlight. |
| packages/app/ui/components/AudioRecorder/Waveform.tsx | Rescales playhead over rendered candle count and allows filler candles to highlight past the playhead. |
| packages/app/ui/components/AudioRecorder/Waveform.web.tsx | Mirrors native waveform logic changes for web (SVG) rendering. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const scaledCandlePlaybackPosition = useMemo(() => { | ||
| if ( | ||
| maxVisibleCandleCount == null || | ||
| values.length <= maxVisibleCandleCount | ||
| values.length === maxVisibleCandleCount | ||
| ) { |
There was a problem hiding this comment.
Added the guard in ea1e2f5 — values.length === 0 short-circuits to the raw position in both Waveform.tsx and Waveform.web.tsx (the caller's position is already 0 for an empty preview, so no NaN reaches the candle comparison).
| const scaledCandlePlaybackPosition = useMemo(() => { | ||
| if ( | ||
| maxVisibleCandleCount == null || | ||
| values.length <= maxVisibleCandleCount | ||
| values.length === maxVisibleCandleCount | ||
| ) { |
There was a problem hiding this comment.
Same guard added to the web variant in ea1e2f5.
Summary
The voice memo waveform highlight didn't line up with the actual playback position — it sat a few candles left of the true position and never filled the waveform at the end of a memo. Noticed while working on #5916 (waveform seek), but independent of it: the misalignment is visible during plain playback.
Changes
How did I test?
Risks and impact
Rollback plan
Revert these commits.
Screenshots / videos
Before — highlight lags and never reaches the end:
5917-before.mp4
After — highlight tracks playback and fills the waveform:
5917-after.mp4
Web — candle state sampled live from the DOM during playback, rendered for visibility (the long parked stretch is the memo buffering):
web-validation.mp4