-
-
Notifications
You must be signed in to change notification settings - Fork 666
feat(record-tour): save and load tour setup as a JSON file #898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
124486d
b66f9f7
9df89ed
756276d
158dc88
8e089ac
7eea2b6
d52671f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,6 +5,8 @@ import { | |||||||||||||||||||||||||
| ArrowUp, | ||||||||||||||||||||||||||
| Camera, | ||||||||||||||||||||||||||
| Circle, | ||||||||||||||||||||||||||
| Download, | ||||||||||||||||||||||||||
| FolderOpen, | ||||||||||||||||||||||||||
| GripHorizontal, | ||||||||||||||||||||||||||
| MapPin, | ||||||||||||||||||||||||||
| Plus, | ||||||||||||||||||||||||||
|
|
@@ -15,11 +17,23 @@ import { | |||||||||||||||||||||||||
| } from "lucide-react"; | ||||||||||||||||||||||||||
| import { useRef, useState } from "react"; | ||||||||||||||||||||||||||
| import { useTranslation } from "react-i18next"; | ||||||||||||||||||||||||||
| import { saveBinaryFileWithFallback } from "../../lib/tauri-io"; | ||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||
| openLocalDataFileWithFallback, | ||||||||||||||||||||||||||
| saveBinaryFileWithFallback, | ||||||||||||||||||||||||||
| saveTextFileWithFallback, | ||||||||||||||||||||||||||
| } from "../../lib/tauri-io"; | ||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||
| DEFAULT_FPS, | ||||||||||||||||||||||||||
| DEFAULT_SEGMENT_SECONDS, | ||||||||||||||||||||||||||
| estimateTourDurationMs, | ||||||||||||||||||||||||||
| isTourRecordingSupported, | ||||||||||||||||||||||||||
| MAX_FPS, | ||||||||||||||||||||||||||
| MAX_SEGMENT_SECONDS, | ||||||||||||||||||||||||||
| MIN_FPS, | ||||||||||||||||||||||||||
| MIN_SEGMENT_SECONDS, | ||||||||||||||||||||||||||
| parseTourConfig, | ||||||||||||||||||||||||||
| recordTour, | ||||||||||||||||||||||||||
| serializeTourConfig, | ||||||||||||||||||||||||||
| type TourKeyframe, | ||||||||||||||||||||||||||
| TourRecordingUnsupportedError, | ||||||||||||||||||||||||||
| } from "../../lib/tour-recorder"; | ||||||||||||||||||||||||||
|
|
@@ -35,12 +49,10 @@ interface RecordTourDialogProps { | |||||||||||||||||||||||||
| type Status = "idle" | "recording" | "ready" | "saving"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const DEFAULT_FILE_NAME = "map-tour"; | ||||||||||||||||||||||||||
| const DEFAULT_FPS = 30; | ||||||||||||||||||||||||||
| const MIN_FPS = 10; | ||||||||||||||||||||||||||
| const MAX_FPS = 60; | ||||||||||||||||||||||||||
| const DEFAULT_SEGMENT_SECONDS = 4; | ||||||||||||||||||||||||||
| const MIN_SEGMENT_SECONDS = 0.5; | ||||||||||||||||||||||||||
| const MAX_SEGMENT_SECONDS = 30; | ||||||||||||||||||||||||||
| // Default leaf name for the saved tour *configuration* (the editable JSON), | ||||||||||||||||||||||||||
| // distinct from the recorded video's name so the two exports are easy to tell | ||||||||||||||||||||||||||
| // apart in a downloads folder. | ||||||||||||||||||||||||||
| const DEFAULT_CONFIG_FILE_NAME = "map-tour-setup"; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| function createId(): string { | ||||||||||||||||||||||||||
| if (typeof crypto !== "undefined" && "randomUUID" in crypto) { | ||||||||||||||||||||||||||
|
|
@@ -92,6 +104,9 @@ export function RecordTourDialog({ | |||||||||||||||||||||||||
| const [error, setError] = useState<string | null>(null); | ||||||||||||||||||||||||||
| const [savedName, setSavedName] = useState<string | null>(null); | ||||||||||||||||||||||||||
| const [saveCancelled, setSaveCancelled] = useState(false); | ||||||||||||||||||||||||||
| // Outcome banner for the configuration save/load actions (kept separate from | ||||||||||||||||||||||||||
| // the video save banner so the two messages never clobber each other). | ||||||||||||||||||||||||||
| const [configMessage, setConfigMessage] = useState<string | null>(null); | ||||||||||||||||||||||||||
| // The finished recording, held until the user names it and clicks Save. | ||||||||||||||||||||||||||
| const [pendingBlob, setPendingBlob] = useState<Blob | null>(null); | ||||||||||||||||||||||||||
| const [fileName, setFileName] = useState(DEFAULT_FILE_NAME); | ||||||||||||||||||||||||||
|
|
@@ -126,6 +141,7 @@ export function RecordTourDialog({ | |||||||||||||||||||||||||
| const clearResultMessages = () => { | ||||||||||||||||||||||||||
| setSavedName(null); | ||||||||||||||||||||||||||
| setSaveCancelled(false); | ||||||||||||||||||||||||||
| setConfigMessage(null); | ||||||||||||||||||||||||||
| setError(null); | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
|
|
@@ -357,6 +373,69 @@ export function RecordTourDialog({ | |||||||||||||||||||||||||
| setStatus("idle"); | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Export the editable tour setup (keyframes, durations, FPS) as a JSON file so | ||||||||||||||||||||||||||
| // it can be reloaded and refined later, independent of the recorded video. | ||||||||||||||||||||||||||
| const handleSaveConfig = async () => { | ||||||||||||||||||||||||||
| if (keyframes.length === 0) return; | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| const content = serializeTourConfig(keyframes, fps); | ||||||||||||||||||||||||||
| const fileType = t("recordTour.configFileType"); | ||||||||||||||||||||||||||
| const name = await saveTextFileWithFallback(content, { | ||||||||||||||||||||||||||
| defaultName: `${DEFAULT_CONFIG_FILE_NAME}.json`, | ||||||||||||||||||||||||||
| filters: [{ name: fileType, extensions: ["json"] }], | ||||||||||||||||||||||||||
| browserTypes: [ | ||||||||||||||||||||||||||
| { description: fileType, accept: { "application/json": [".json"] } }, | ||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||
| mimeType: "application/json", | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| // Cancelling the dialog returns null and is a no-op, so only clear a prior | ||||||||||||||||||||||||||
| // result banner once the file is actually written. | ||||||||||||||||||||||||||
| if (name) { | ||||||||||||||||||||||||||
| clearResultMessages(); | ||||||||||||||||||||||||||
| setConfigMessage(t("recordTour.configSaved", { name })); | ||||||||||||||||||||||||||
|
Comment on lines
+394
to
+395
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug —
A user who records a tour, saves the video ("Saved as map-tour.webm"), and then immediately clicks "Save setup" will see the video-saved banner silently disappear even though nothing in the tour changed. The fix is to clear only the states that are directly relevant here (the previous config banner and any lingering load/save error) while leaving
Suggested change
(The new |
||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||
| console.warn("Tour configuration save failed", err); | ||||||||||||||||||||||||||
| clearResultMessages(); | ||||||||||||||||||||||||||
| setError(t("recordTour.configSaveError")); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // Load a previously saved tour setup, replacing the current keyframe list and | ||||||||||||||||||||||||||
| // frame rate. Fresh ids are minted so reloaded rows never collide. A bad file | ||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Quality (low confidence):
The existing codebase's This is a deliberate tradeoff and the current implementation is safe — just flagging it for the design review pass. |
||||||||||||||||||||||||||
| // throws a parse error, surfaced as a translated message rather than a crash. | ||||||||||||||||||||||||||
| const handleLoadConfig = async () => { | ||||||||||||||||||||||||||
| // Loading replaces the whole tour, so confirm first when there is existing | ||||||||||||||||||||||||||
| // work a misclick would otherwise wipe. | ||||||||||||||||||||||||||
| if (keyframes.length > 0 && !window.confirm(t("recordTour.confirmLoad"))) { | ||||||||||||||||||||||||||
|
giswqs marked this conversation as resolved.
giswqs marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The existing
Suggested change
(The iframe-detection heuristic is imperfect; a proper shadcn |
||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||
| const fileType = t("recordTour.configFileType"); | ||||||||||||||||||||||||||
| const result = await openLocalDataFileWithFallback({ | ||||||||||||||||||||||||||
| filters: [{ name: fileType, extensions: ["json"] }], | ||||||||||||||||||||||||||
| accept: ".json,application/json", | ||||||||||||||||||||||||||
| readText: true, | ||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||
| // Only a null result means the picker was cancelled (a no-op). An empty | ||||||||||||||||||||||||||
| // file still flows through so parseTourConfig surfaces a real error rather | ||||||||||||||||||||||||||
| // than silently doing nothing after the user explicitly chose a file. | ||||||||||||||||||||||||||
| if (result == null) return; | ||||||||||||||||||||||||||
|
giswqs marked this conversation as resolved.
|
||||||||||||||||||||||||||
| clearResultMessages(); | ||||||||||||||||||||||||||
| const config = parseTourConfig(result.text ?? ""); | ||||||||||||||||||||||||||
| setKeyframes(config.keyframes.map((kf) => ({ ...kf, id: createId() }))); | ||||||||||||||||||||||||||
| setFps(config.fps); | ||||||||||||||||||||||||||
| setFpsText(String(config.fps)); | ||||||||||||||||||||||||||
| setConfigMessage( | ||||||||||||||||||||||||||
| t("recordTour.configLoaded", { count: config.keyframes.length }), | ||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||
| console.warn("Tour configuration load failed", err); | ||||||||||||||||||||||||||
| clearResultMessages(); | ||||||||||||||||||||||||||
| setError(t("recordTour.configLoadError")); | ||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||
|
giswqs marked this conversation as resolved.
|
||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| const totalSeconds = estimateTourDurationMs(keyframes) / 1000; | ||||||||||||||||||||||||||
| const canRecord = | ||||||||||||||||||||||||||
| keyframes.length >= 2 && RECORDING_SUPPORTED && status === "idle"; | ||||||||||||||||||||||||||
|
|
@@ -411,6 +490,33 @@ export function RecordTourDialog({ | |||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| {/* Save / load the editable tour setup so work can be paused, resumed, | ||||||||||||||||||||||||||
| and reused across sessions (independent of the recorded video). */} | ||||||||||||||||||||||||||
| <div className="flex items-center gap-2"> | ||||||||||||||||||||||||||
| <Button | ||||||||||||||||||||||||||
| type="button" | ||||||||||||||||||||||||||
| variant="outline" | ||||||||||||||||||||||||||
| size="sm" | ||||||||||||||||||||||||||
| className="flex-1" | ||||||||||||||||||||||||||
| disabled={editingFrozen} | ||||||||||||||||||||||||||
| onClick={handleLoadConfig} | ||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||
| <FolderOpen className="mr-1.5 h-3.5 w-3.5" /> | ||||||||||||||||||||||||||
| {t("recordTour.loadConfig")} | ||||||||||||||||||||||||||
| </Button> | ||||||||||||||||||||||||||
| <Button | ||||||||||||||||||||||||||
| type="button" | ||||||||||||||||||||||||||
| variant="outline" | ||||||||||||||||||||||||||
| size="sm" | ||||||||||||||||||||||||||
| className="flex-1" | ||||||||||||||||||||||||||
| disabled={editingFrozen || keyframes.length === 0} | ||||||||||||||||||||||||||
| onClick={handleSaveConfig} | ||||||||||||||||||||||||||
| > | ||||||||||||||||||||||||||
| <Download className="mr-1.5 h-3.5 w-3.5" /> | ||||||||||||||||||||||||||
| {t("recordTour.saveConfig")} | ||||||||||||||||||||||||||
| </Button> | ||||||||||||||||||||||||||
| </div> | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| <div className="flex items-center justify-between gap-2"> | ||||||||||||||||||||||||||
| <Button | ||||||||||||||||||||||||||
| type="button" | ||||||||||||||||||||||||||
|
|
@@ -510,6 +616,11 @@ export function RecordTourDialog({ | |||||||||||||||||||||||||
| {t("recordTour.saveCancelled")} | ||||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||
| {configMessage && ( | ||||||||||||||||||||||||||
| <p className="text-sm text-emerald-600 dark:text-emerald-400"> | ||||||||||||||||||||||||||
| {configMessage} | ||||||||||||||||||||||||||
| </p> | ||||||||||||||||||||||||||
| )} | ||||||||||||||||||||||||||
| {error && <p className="text-sm text-destructive">{error}</p>} | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| {busy ? ( | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1118,6 +1118,9 @@ export async function openLocalDataFileWithFallback( | |
| reject(error); | ||
| } | ||
| }; | ||
| // Resolve (rather than hang) when the dialog is dismissed without a pick; | ||
| // `change` never fires on cancel, so without this the Promise never settles. | ||
| input.addEventListener("cancel", () => resolve(null)); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good fix for the hanging-promise bug. Worth noting: the |
||
| input.click(); | ||
| }); | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: comment overstates the separation guarantee
The comment says the config banner is "kept separate from the video save banner so the two messages never clobber each other". In fact they can coexist in the DOM simultaneously (good), but both are erased by
clearResultMessages()whenever any new operation starts — so a "Saved setup as…" banner does disappear if the user immediately starts a recording.The comment would be more accurate as: "kept separate so both can display simultaneously; cleared together when any new operation begins."
This is cosmetic only.