Skip to content

Commit 44d7fa1

Browse files
authored
feat(record-tour): clearer keyframe layout, recapture, and two-step save (#886)
* feat(record-tour): clearer keyframe layout, recapture, and a two-step save Address the Record Map Tour UX report (#874): - Lay each keyframe out over two rows so the transition control gets its own line with a full "Transition N seconds" label that no longer clips in a narrow panel. - Add a Recapture (camera) action to every keyframe row so a stop can be re-framed in place from the current map view, instead of deleting and re-adding it (which lost its order and segment duration). - Decouple recording from saving: a finished take is held in memory and the user names it and clicks Save (or Discard to re-record), rather than an automatic map-tour.webm download firing the instant capture ends. The custom name flows through to both the native Save As dialog and the browser download fallback. Fixes #874 * Address Claude review feedback - Surface the discard consequence in the "ready" UI: the message now warns that editing the tour discards the captured recording, so the silent drop when a keyframe is edited is no longer a hidden footgun. - Use a dedicated recordTour.saveError ("Could not save the video.") in the handleSave catch instead of recordError, which wrongly blamed the recording step for a file-write failure. - autoFocus the file name input when the panel switches to the save step, and reset the name to the default after a successful save or a discard so a second recording doesn't show the previous name. * Address Claude review feedback (round 2) - Freeze the keyframe-editing controls (Add current view, recapture, reorder, remove, segment duration) while a finished take is held in the "ready" state, so the recording can only be dropped via the explicit Discard button rather than a one-click edit whose footer warning may have scrolled off-screen. The ready-state message reverts to "discard to re-record" accordingly. - Guard handleSave with a savingRef so a fast double-click or Enter keyboard repeat can't fire two concurrent saves (two pickers / two downloads) before the "saving" state re-renders. * Address Claude review feedback: avoid re-copying the recording on save - Widen saveBinaryFileWithFallback (and saveBinaryFileBrowser) to accept Blob | Uint8Array. The browser path now writes a Blob straight through, and the Tauri path materializes bytes only after the save dialog is confirmed. - RecordTourDialog passes the recorded Blob directly instead of copying it into a Uint8Array on every handleSave call, so a cancel-and-retry no longer re-copies a potentially several-hundred-MB recording. Existing Uint8Array callers are unaffected (backward-compatible union type). * Address Claude review feedback (round 3) - Freeze the FPS input while a recording is held (status "ready"), matching the other editing controls: a captured take's frame rate is already fixed, so editing FPS then would only mislead. - Keep the fly-to preview available in the "ready" state via a separate previewDisabled prop gated on busy alone. Previewing only moves the map camera and can't produce a stale recording, so users can verify the tour before choosing Save or Discard without having to discard first. * Address Claude review feedback (round 4) - Disable the panel close button while a finished take is held ("ready"), using editingFrozen instead of busy, so closing can't silently strand a recording in memory or drop the user back into the save step on reopen. Save or discard first. - Document that saveBinaryFileWithFallback uses a Blob's own .type as-is and only applies options.mimeType when wrapping a Uint8Array, so the contract is explicit for future callers. * Address Claude review feedback (round 5) - Pluralize the transition-duration unit so a 1-second segment reads "Transition 1 second" instead of "1 seconds", using i18next count plurals (secondsLong_one / secondsLong_other). - Ignore Enter key-repeat in the file name field (!event.repeat) so holding Enter doesn't re-invoke handleSave on every repeated keydown. * Address Claude review feedback (round 6) - Stop clearing the "Saved …" / outcome banner on plain keyframe edits. The per-edit invalidateRecording() was dead for its real purpose (editing is frozen while a take is held, so it never had a pending blob to drop) and only caused a regression: clicking into a duration field wiped the save confirmation. Replaced it with a narrower clearResultMessages() called only from addCurrentView (its original pre-PR behavior); recapture/remove/move/ duration no longer touch the banner. - Add a finally safety net in handleRecord so an exception escaping the catch (e.g. t() throwing) can't leave the dialog stuck on "recording" with a null abort controller; it falls back to "idle". - Log the raw error in handleSave's catch (console.warn) so a save failure is debuggable behind the user-facing message. - Note that arrayBuffer() rejects on a detached Blob and the rejection propagates to the caller's catch. * Address Claude review feedback (round 7) - Clear the "Saved …" outcome banner when recapturing a keyframe, matching addCurrentView: both change a keyframe's camera, so the saved video no longer matches the tour and the success banner would otherwise mislead. Plain reorder/duration tweaks still leave the banner alone. * Address Claude review feedback (round 8) - Pluralize the transition unit against the value being typed (falling back to the committed value), so the label tracks the input and reads "1 second" rather than "1 seconds" mid-edit before blur commits. - Correct an inaccurate comment: Blobs have no "detached" state (that's ArrayBuffer); arrayBuffer() can still reject for other reasons (OOM, unavailable backing store), which propagates to the caller's catch. * Address CodeRabbit review feedback: clear banner on every real edit Make the "Saved …" banner behavior fully consistent: every edit that actually changes the tour now clears it, while no-op interactions leave it alone. - removeKeyframe clears the banner (dropping a stop changes the recording), per CodeRabbit's note. - move clears only on an in-range reorder (guards the inert end-of-list click). - setSegmentSeconds clears only when the duration actually changes, so merely focusing and blurring a field (which re-commits the same value) no longer wipes the banner — preserving the earlier no-op-blur fix.
1 parent 395e5a9 commit 44d7fa1

3 files changed

Lines changed: 359 additions & 114 deletions

File tree

0 commit comments

Comments
 (0)