feat: tap or drag voice memo waveform to seek#5916
Conversation
janicduplessis
force-pushed
the
@janic/voice-memo-waveform-seek
branch
from
c893e29 to
d8c8cec
Compare
![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: 3cd9bc46b2
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c94872c9ce
ℹ️ 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".
| const duration = | ||
| progress?.loadState === 'loaded' && isThisSourceLoaded | ||
| ? progress.duration | ||
| : block.voiceMemo.duration ?? 0; |
There was a problem hiding this comment.
When expo-audio reports a loaded status before it has determined the duration (duration === 0), this branch chooses progress.duration even though block.voiceMemo.duration is available, and the zero-duration guard immediately returns. In that scenario, tapping or dragging the waveform stops seeking as soon as the memo is loaded; the unloaded path already uses the metadata duration, so it should also be used as a fallback here.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 63fcc96 — the loaded path now uses progress.duration only when it's > 0 and otherwise falls back to block.voiceMemo.duration, same as the unloaded path. (Defensive fix verified by review; the transient loaded-with-zero-duration state isn't deterministically reproducible in manual testing.)
There was a problem hiding this comment.
Follow-up: validated on the iOS sim by temporarily forcing the loaded duration to 0 in the working tree — with the fallback in place, tapping the waveform at ~70% during playback still seeks (timer jumps accordingly); without it the zero-duration guard swallowed the seek.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63fcc968e8
ℹ️ 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".
| loadSource() | ||
| .then(() => { | ||
| if (pendingSeekRef.current != null) { | ||
| nowPlaying.seekTo(pendingSeekRef.current); |
There was a problem hiding this comment.
When an unloaded memo finishes loading while the user is still dragging, this calls nowPlaying.seekTo(...) but does not return/await its promise, so the following .finally clears isLoadingForSeekRef and pendingSeekRef before the native seek completes and before the synthetic progress event can mark this source as loaded. A subsequent pan update in that gap still sees isThisSourceLoaded === false and can start another replace() for the same URL, reintroducing the multiple-load race this guard is meant to prevent. The fresh evidence is that the post-load path drops the seek promise, so the guard is released before the source becomes observable as loaded/seeked.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in e6fc6a7, two layers: the .then now returns the nowPlaying.seekTo(...) promise so the .finally holds isLoadingForSeekRef until the post-load seek actually completes, and the direct-seek branch also accepts nowPlaying.nowPlaying?.url === sourceUri — the provider sets that synchronously when a load resolves, which covers the remaining gap before the progress event re-renders isThisSourceLoaded. Validated on the iOS sim with a temporary counter in replace(): four continuous scrub passes spanning the load of an unloaded memo produced exactly one replace(), ending parked at the final drag position.
There was a problem hiding this comment.
Pull request overview
Adds waveform-based seeking for voice memos (tap-to-seek and drag-to-scrub) by wiring gesture input to the existing now-playing audio controller, and fixes progress reporting so UI updates immediately after seeks (including while paused).
Changes:
- Add tap + horizontal pan gestures to the voice memo waveform and map gesture X to a seek time.
- Update now-playing progress handling: remove web ms→s conversion and emit synthetic progress after
seekTo. - Deduplicate concurrent
replace()loads so play/seek can share a single in-flight load.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/app/ui/contexts/nowPlaying.tsx | Updates progress units, emits synthetic progress on seek, and adds shared in-flight load logic plus seek/scrub controller APIs. |
| packages/app/ui/components/PostContent/BlockRenderer.tsx | Adds gesture handling around the voice memo waveform and computes seek time from gesture position. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const inFlightLoadRef = useRef<Promise<void> | null>(null); | ||
| const loadSource = useCallback(() => { | ||
| if (sourceUri == null) { | ||
| return Promise.reject(new Error('No source to load')); | ||
| } | ||
| if (inFlightLoadRef.current == null) { | ||
| inFlightLoadRef.current = nowPlaying | ||
| .replace({ url: sourceUri }) | ||
| .finally(() => { | ||
| inFlightLoadRef.current = null; | ||
| }); | ||
| } | ||
| return inFlightLoadRef.current; | ||
| }, [nowPlaying, sourceUri]); |
| if (isThisSourceLoaded || nowPlaying.nowPlaying?.url === sourceUri) { | ||
| nowPlaying.seekTo(seconds); | ||
| return; | ||
| } |
| // the Skia canvas would otherwise capture touches meant for | ||
| // the seek gesture | ||
| style={{ height: 22, pointerEvents: 'none' }} | ||
| /> |
| const candleSize = WAVEFORM_CANDLE_WIDTH + WAVEFORM_CANDLE_SPACING; | ||
| const drawnExtent = Math.floor(width / candleSize) * candleSize; | ||
| if (drawnExtent <= 0) { |
Summary
Tapping the waveform on a voice memo did nothing on iOS and Android — there was no way to seek within a memo. This adds tap-to-seek and drag-to-scrub on the voice memo waveform. Found while migrating audio playback away from
expo-av.Related: #5917 independently fixes the waveform highlight alignment during playback.
Changes
VoiceMemoBlockwraps the waveform in aGestureDetector(tap + horizontal pan) that turns the touch position into a fraction of the memo's duration and seeks the player. TheseekToAPI already existed on the now-playing context but was never hooked up to any UI. Pan seeks are throttled to 100ms so scrubbing doesn't spam the native player, and the pan only activates on horizontal drags so vertical channel scrolling still works.pointerEvents: 'none'(same workaroundAudioRecorderuses for its waveform).NowPlayingProvider.seekTonow sends a progress update right after seeking so the timer and waveform highlight move immediately — expo-audio doesn't send progress updates while paused.currentTime/durationin seconds (plainHTMLMediaElementunits), but the progress pipeline divided by 1000 as if they were milliseconds — so the timer always read 0:00 and seeking a loaded memo landed at ~0. The conversion is removed.Seeking a memo that hasn't been played yet loads it (paused) with the playhead at the tapped position, so play starts from there. While the load is running, only the latest requested position is kept, so scrubbing an unloaded memo can't kick off multiple loads.
How did I test?
Risks and impact
Rollback plan
Revert these commits.
Screenshots / videos
5916-demo.mp4