Component: apps/web/components/review/compare/compare-overlay.tsx (video compare / wipe-and-scrubber overlay, introduced in PR #169)
Repro
- Open the compare overlay on a video asset with 3+ ready versions (e.g. v1, v2, v3).
- Compare v1 (left/A) vs v2 (right/B).
- Use the offset steppers in the scrubber to nudge A and/or B (
offA/offB) to align the two clips (e.g. because v2 has a few extra frames of pre-roll).
- Without clicking the "Sync" / reset-offsets button, switch either side's version via its
CompareVersionSelect dropdown — e.g. change the left pane from v1 to v3.
- Observe: the new pairing (v3 vs v2) plays back with the stale offset(s) from the v1/v2 comparison still applied, so the two clips are silently mis-synced. Nothing in the UI indicates the offsets are stale/carried over.
Root cause
offA/offB are read straight from URL search params and fed into timingA/timingB:
// compare-overlay.tsx:87-88
const offA = parseOffsetParam(searchParams.get('offA'))
const offB = parseOffsetParam(searchParams.get('offB'))
The two version-select dropdowns' onChange handlers only update which version is shown — they never touch the offset params:
// compare-overlay.tsx:313 (left/A version switch)
onChange={(v) => writeParams((p) => p.set('compare', v.id))}
// compare-overlay.tsx:341 (right/B version switch)
onChange={(v) => setCurrentVersion(v)}
CompareVersionSelect itself (compare-version-select.tsx:72-76) is a plain controlled dropdown that just invokes the onChange prop it's given — it has no concept of offsets, so there's no hidden reset happening there either.
The only code path that clears offA/offB is the explicit "Sync" button in the scrubber:
// compare-overlay.tsx:499-501
onResetOffsets={() =>
writeParams((p) => { p.delete('offA'); p.delete('offB') })
}
wired to the manual reset button in compare-scrubber.tsx:229-236 (title="Reset offsets — re-sync both sides").
So any offset dialed in for one version pairing silently carries over to whatever new pairing the user selects next, with no reset and no warning.
Suggested fix
Clear offA/offB whenever the identity of either compared version changes, not just when the user explicitly clicks Sync. E.g. in the two onChange handlers at compare-overlay.tsx:313 and :341, also delete offA/offB from the params before setting the new version:
onChange={(v) => writeParams((p) => { p.set('compare', v.id); p.delete('offA'); p.delete('offB') })}
and for the right/B side, similarly clear the offset params (via writeParams, or by having setCurrentVersion's caller also clear them) before/along with setCurrentVersion(v). Alternatively, key offA/offB off the pair (left.id, right.id) so a URL replace / effect resets them automatically whenever either id changes.
Component:
apps/web/components/review/compare/compare-overlay.tsx(video compare / wipe-and-scrubber overlay, introduced in PR #169)Repro
offA/offB) to align the two clips (e.g. because v2 has a few extra frames of pre-roll).CompareVersionSelectdropdown — e.g. change the left pane from v1 to v3.Root cause
offA/offBare read straight from URL search params and fed intotimingA/timingB:The two version-select dropdowns'
onChangehandlers only update which version is shown — they never touch the offset params:CompareVersionSelectitself (compare-version-select.tsx:72-76) is a plain controlled dropdown that just invokes theonChangeprop it's given — it has no concept of offsets, so there's no hidden reset happening there either.The only code path that clears
offA/offBis the explicit "Sync" button in the scrubber:wired to the manual reset button in
compare-scrubber.tsx:229-236(title="Reset offsets — re-sync both sides").So any offset dialed in for one version pairing silently carries over to whatever new pairing the user selects next, with no reset and no warning.
Suggested fix
Clear
offA/offBwhenever the identity of either compared version changes, not just when the user explicitly clicks Sync. E.g. in the twoonChangehandlers at compare-overlay.tsx:313 and :341, also deleteoffA/offBfrom the params before setting the new version:and for the right/B side, similarly clear the offset params (via
writeParams, or by havingsetCurrentVersion's caller also clear them) before/along withsetCurrentVersion(v). Alternatively, keyoffA/offBoff the pair(left.id, right.id)so a URL replace / effect resets them automatically whenever either id changes.