Commit 2c55c31
authored
feat(record-tour): save and load tour setup as a JSON file (#898)
* feat(record-tour): save and load tour setup as a JSON file
The Record Map Tour panel could only export the recorded video; the
underlying keyframes, durations, and frame rate were lost when the panel
closed, so a tour could not be paused, refined, or reused.
Add a Save setup / Load setup pair near the top of the panel. Save writes
the keyframes and FPS to a JSON file; Load reads one back, repopulating the
keyframe list and frame rate (fresh ids are minted so reloaded rows never
collide). The serializer and parser share the FPS/segment bounds with the
controls, so a hand-edited or stale file is clamped to the supported range,
and a malformed file surfaces a translated error instead of crashing.
Closes #897
* Address Claude review feedback
- parseTourConfig now rejects a config written by a newer, incompatible
format version (was stamped on save but never read back); a missing or
older version is still accepted.
- Clamp zoom/pitch/bearing into MapLibre's supported ranges on load to match
the documented validation contract (only durationMs/fps were clamped).
- Rename the config file default to "map-tour-setup" so it is genuinely
distinct from the video name, and fix the misleading comment.
- Grammar: "Saved setup as {{name}}".
- Add tests for camera clamping and newer-version rejection.
* Address Claude review feedback (round 2)
- Wrap bearing onto (-180, 180] instead of clamping, so a hand-edited 270
maps to -90 (west) rather than 180 (south).
- Reject a keyframe whose latitude is outside ±90 (a real out-of-range
coordinate), matching the validation the comment claims.
- Cap parsed keyframes at 500 so a crafted/huge file can't make the parser
allocate a giant array and loop createId() over it.
- handleLoadConfig: only clear the result banner once a file is actually
chosen, so cancelling the picker no longer wipes a prior "Saved setup…".
- Confirm before loading when the panel already has keyframes, so a misclick
on "Load setup" can't silently discard in-progress work (new confirmLoad
string).
- Tests for bearing wrap, latitude rejection, and the keyframe cap.
* Address CodeRabbit review feedback
- Guard the raw config text length (1 MB) before JSON.parse, so a pathological
file is rejected without being fully allocated first (completes the
MAX_KEYFRAMES DoS hardening, which only ran post-parse).
* Address Claude review feedback (round 3)
- handleSaveConfig now clears the result banner only after the file is
actually written, matching handleLoadConfig, so cancelling the save dialog
no longer wipes a prior "Saved setup…" message.
- handleLoadConfig short-circuits only on a cancelled picker (null result);
an empty file now flows through to parseTourConfig so it surfaces a real
error instead of silently doing nothing.
* Address Claude review feedback (round 4)
- Clamp each keyframe's durationMs on serialize too, mirroring parseKeyframe,
so save/load is symmetric and a programmatic caller can't persist an
out-of-range duration.
- Tighten the version gate to also reject a present-but-non-numeric version
(e.g. "2"); a missing version is still accepted as legacy v1. Add tests for
the string-version rejection and the missing-version acceptance.
* Address Claude review feedback (round 5)
- Fix openLocalDataFileWithFallback hanging forever when the browser file
picker is dismissed without a selection: the input only had an onchange
handler (which never fires on cancel), so handleLoadConfig's await never
settled. Add a "cancel" listener that resolves null, matching the existing
pickImageFilesWithFallback pattern.
- Assert the missing-fps fallback to DEFAULT_FPS in the parse test.
* Address Claude review feedback (round 6)
- Tighten the version gate: a present version must be an integer in
[1, TOUR_CONFIG_VERSION], so an unrecognized 0 or negative value is now
rejected too (only a missing version still defaults to v1). Add a test for
the version-below-1 case.1 parent 9a9f62c commit 2c55c31
5 files changed
Lines changed: 544 additions & 7 deletions
File tree
- apps/geolibre-desktop/src
- components/layout
- i18n/locales
- lib
- tests
Lines changed: 118 additions & 7 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5 | 5 | | |
6 | 6 | | |
7 | 7 | | |
| 8 | + | |
| 9 | + | |
8 | 10 | | |
9 | 11 | | |
10 | 12 | | |
| |||
15 | 17 | | |
16 | 18 | | |
17 | 19 | | |
18 | | - | |
19 | 20 | | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
20 | 28 | | |
21 | 29 | | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
22 | 35 | | |
| 36 | + | |
23 | 37 | | |
24 | 38 | | |
25 | 39 | | |
| |||
35 | 49 | | |
36 | 50 | | |
37 | 51 | | |
38 | | - | |
39 | | - | |
40 | | - | |
41 | | - | |
42 | | - | |
43 | | - | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
44 | 56 | | |
45 | 57 | | |
46 | 58 | | |
| |||
92 | 104 | | |
93 | 105 | | |
94 | 106 | | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
95 | 110 | | |
96 | 111 | | |
97 | 112 | | |
| |||
126 | 141 | | |
127 | 142 | | |
128 | 143 | | |
| 144 | + | |
129 | 145 | | |
130 | 146 | | |
131 | 147 | | |
| |||
357 | 373 | | |
358 | 374 | | |
359 | 375 | | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
360 | 439 | | |
361 | 440 | | |
362 | 441 | | |
| |||
411 | 490 | | |
412 | 491 | | |
413 | 492 | | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
414 | 520 | | |
415 | 521 | | |
416 | 522 | | |
| |||
510 | 616 | | |
511 | 617 | | |
512 | 618 | | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
513 | 624 | | |
514 | 625 | | |
515 | 626 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
479 | 479 | | |
480 | 480 | | |
481 | 481 | | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
482 | 491 | | |
483 | 492 | | |
484 | 493 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1118 | 1118 | | |
1119 | 1119 | | |
1120 | 1120 | | |
| 1121 | + | |
| 1122 | + | |
| 1123 | + | |
1121 | 1124 | | |
1122 | 1125 | | |
1123 | 1126 | | |
| |||
0 commit comments