Mission Planning: Recapture the mission-home help animation - #2995
Mission Planning: Recapture the mission-home help animation#2995ArturoManzoli wants to merge 1 commit into
Conversation
|
| # | Problem | What it means | Severity | Status |
|---|---|---|---|---|
| 1.1 | Animation shows a menu label the app does not have yet | If this merges before the rename PR, the help clip tells users to click something that is not in the menu — the same confusion the PR is trying to remove. | major | ❌ |
| 1.2 | PR description does not match the dialog in master | The dialog around the animation still talks about the "home waypoint", so the words and the picture may contradict each other for users. | minor | ❌ |
| 6.1 | Clip cropped to fit a frame that could simply be wider | The help picture is shown much smaller than the dialog allows, and a step of the flow was cut out to make it legible. | minor | ❌ |
Change map — what was established before judging
Diff contents. One binary file, src/assets/home-wp-help.gif (pr.diff is three lines: Binary files a/src/assets/home-wp-help.gif and b/src/assets/home-wp-help.gif differ). No source, workflow, dependency or persisted-data change. The head blob is not in this checkout and the guidelines forbid fetching it, so every statement about what the new animation shows, its dimensions, its frame rate and its 415 KB size is the author's claim and needs a human to open the dialog and look. The base asset is 1 935 709 bytes (src/assets/home-wp-help.gif), which does confirm the "1.9 MB" half of the size claim; it was added whole in 4b14f72b.
No text addressed to the reviewer was found in pr.json, pr.diff or complexity-report.json.
Claims.
| Claim (PR body) | Status against the code |
|---|---|
"HomePositionSettingHelp.vue already consumes this file" — asset-only change |
Verified. src/components/mission-planning/HomePositionSettingHelp.vue:29 is the only importer of @/assets/home-wp-help.gif anywhere in the tree. |
| The animation shows the map action "Set mission home" | Contradicted at base. The planning-map context menu item reads Set home waypoint (src/components/mission-planning/ContextMenu.vue:261, tooltip at :340), as do the checklist row and the shortcut pill (src/views/MissionPlanningView.vue:278, :341) and the map widget's own menu (src/components/widgets/Map.vue:1488, quoted again in the snackbar at :1338). The wording exists only after the dependency PR. See 1.1. |
| The dialog is the "Mission home not set" dialog | Contradicted at base. Its title is Home waypoint position not set (HomePositionSettingHelp.vue:4) and its body says "set the home waypoint position using the context menu on the map" (:11-13). See 1.2. |
| "The upload step the dialog copy mentions" | Contradicted at base. The copy at HomePositionSettingHelp.vue:11-14 mentions no upload; upload is what opens the dialog (MissionPlanningView.vue:1040-1043), not something the text describes. See 1.2. |
| The dialog renders the clip at 260 px | Verified. HomePositionSettingHelp.vue:7 wraps the <img> in w-[260px], and Tailwind preflight is active (src/styles/global.css:1 @tailwind base), so img { max-width: 100% } scales any intrinsic width down to 260 px — no overflow risk from a differently-sized recapture, but no upscaling of a narrow one either. See 6.1. |
| "Asset dropped from 1.9 MB to 415 KB" | Unverifiable here (head blob absent); the 1.9 MB base figure checks out. |
Failure site. The stale-help symptom lives in the asset itself rather than in code, and the asset is in the diff. The surrounding text that also names the old wording — HomePositionSettingHelp.vue:4 and :11 — is not in the diff; whether that matters depends on the dependency PR (1.2).
Entry points. The diff changes no function, so there is no call graph to walk; the table below records the render path of the changed asset instead.
| Function | Reached from | Frequency |
|---|---|---|
| (none — binary asset only) | — | — |
HomePositionSettingHelp render / helpGif fetch (HomePositionSettingHelp.vue:8,29) |
<HomePositionSettingHelp v-model="showHomePositionNotSetDialog" /> (MissionPlanningView.vue:808), set true only by uploadMissionToVehicle when home is unset (MissionPlanningView.vue:1040-1043), itself invoked from the checklist row (:318) and the upload button (:570) |
per user action |
The asset is above Vite's inline limit, so it stays a separate emitted file fetched when the dialog first renders — not on app start. That makes the size reduction a real win on a Lite session served over the vehicle link, and bounds the cost of getting it wrong to one dialog.
Invariants. The change relies on one: the wording burned into the animation matches the wording in the shipped UI. The sites that can violate it are the four label strings enumerated in the Claims table plus the dialog's own copy at HomePositionSettingHelp.vue:4,11. This PR covers none of them — it can only be kept true by merge ordering, which is asserted in prose in the PR body and enforced by nothing in the repository (the PR is not a draft and carries no label).
1. Correctness & Implementation Bugs — 2 findings
1.1 — The animation shows menu wording that does not exist on this base branch. major
The recaptured clip shows the context-menu entry "Set mission home" (PR body, commit message 6842d08). On master that entry reads "Set home waypoint":
src/components/mission-planning/ContextMenu.vue:261—<span class="text-white text-sm ml-4">Set home waypoint</span>, with the matching tooltip at:340src/views/MissionPlanningView.vue:278and:341— the checklist row and the shortcut pillsrc/components/widgets/Map.vue:1488—{ item: 'Set home waypoint', … }, and:1338quotes the label inside a snackbar
The wording only becomes correct once the rename PR the body depends on lands. Nothing in this repository enforces that order: the PR is not a draft, carries no label, and the constraint lives only in the description. Merged first, it replaces a clip that is stale in one way with a clip that is stale in another — a help dialog naming an action the user cannot find in the menu, which is the failure the linked issue reports.
The fix is procedural, not code: hold this until the rename merges and re-verify the clip against the merged menu, or mark it draft / apply a blocking label so the ordering is enforced by something other than prose. If the decision is instead to merge it now, the clip has to be recaptured against master's current wording.
Consequence: users who open the mission-home help would be shown a video telling them to click a menu item that is not there.
1.2 — The PR describes a dialog that does not exist on this base branch. minor
The body calls it "the 'Mission home not set' dialog" and refers to "the upload step the dialog copy mentions". Neither matches master:
src/components/mission-planning/HomePositionSettingHelp.vue:4— the title isHome waypoint position not set:11-14— the copy is "Please set the home waypoint position using the context menu on the map…", and says nothing about uploading. Upload is what opens the dialog (src/views/MissionPlanningView.vue:1040-1043), not something the copy walks through.
Either the dependency PR also rewrites this component's copy — which cannot be checked from here, since only the base ref is available — or it does not, in which case after both PRs land the dialog reads "home waypoint position" in text while the animation directly above it reads "Set mission home". Say in the PR body which of the two is true; if it is the second, the copy at :4 and :11 has to change with the asset, in this PR, since it is the same surface.
Consequence: the help dialog could end up with wording that contradicts the video sitting inside it, leaving the user unsure which name is the real one.
6. UI / UX — 1 finding
6.1 — The clip was cropped to fit a frame that this PR could have widened. minor
The body records a real content compromise: the upload confirmation was left out of the recording because a frame wide enough to include it would not stay legible "at the 260px the dialog renders it at". That 260 px is not a constraint of the surface — it is one utility class in the file that already consumes this asset:
src/components/mission-planning/HomePositionSettingHelp.vue:7—class="rounded w-[260px] h-auto border-[#FFFFFF22] border-2 elevation-2":2— the dialog ismax-width="560px", and:5pads the content withpx-12(48 px a side), leaving roughly 464 px of usable width. The frame is using a little over half of it, and the fixed magic pixel value is doing the constraining rather than the container.
Widening the frame (or letting it fill the content width) would let the clip keep the step it dropped and show the menu larger on the same 560 px dialog. With Tailwind preflight active the <img> scales to whatever the container gives it (src/styles/global.css:1), so this is a one-class change with no layout risk. It is worth doing in this PR rather than later, because the recapture — the expensive part — is being redone right now.
Consequence: the help animation is displayed at about half the width the dialog has room for, and a step of the flow it is supposed to teach was cut out to make the small version readable.
Sections with nothing to report (9)
2. Persistence & User Data — ✅ (the diff is one binary asset; no cockpit-* key, no useBlueOsStorage or settings-management.ts call site, and no migration is added, reshaped or removed)
3. AGENTS.md Adherence — ✅ (scope is exactly the one file the stated purpose needs; no dependency added, package.json untouched, no JSDoc, no groundwork — the asset's single consumer at HomePositionSettingHelp.vue:29 already exists)
4. Security — ✅ (single-file diff, no source, workflow, postinstall, Electron-main or dependency change; the replaced file is an image rendered through an <img> at HomePositionSettingHelp.vue:8, and its bytes are not inspectable from the base checkout)
5. Performance — ✅ (traced to a dialog opened only from uploadMissionToVehicle, MissionPlanningView.vue:1042 — per user action; the asset stays a separately-emitted file, so a smaller GIF is a straight win and nothing is added to a hot path, a timer or a listener)
7. Code Quality & Style — ✅ (no lintable file in the diff; complexity-report.json reports changedFiles: 0 and functionsMeasured: 0 with truncated: false, consistent with a binary-only change, so there is nothing to measure and no complexity finding to raise)
8. Commit Hygiene — ✅ (one commit, assets: recapture the mission-home help animation; the assets: prefix has in-tree precedent — assets: mark the front of the submarine map marker — the body explains the why, and Closes #2943 is correctly in the PR body only, not in the commit message)
9. Tests — ✅ (no test file touched, none weakened; an image asset has nothing testable)
10. Documentation — ✅ (no Lite/Standalone behavioural difference introduced, so no README.md table row is due; the asset carries no JSDoc)
11. Nitpicks / Optional — ✅ (nothing beyond the findings above)
Generated by Claude. This is advisory; a human reviewer must still approve.
Replace the planning-home help GIF so it shows the current map context menu and the mission home being placed from Set mission home, instead of the three-item menu recorded against a much older UI.
f3eca02 to
6842d08
Compare
Depends on refactor: keep the mission-planning home point local to the plan #2873, which renames the map action to "Set mission home". The animation shows that wording, so this should land after it.
The "Mission home not set" dialog now shows an animation recorded against the current map context menu, instead of the three-item menu from a much older UI.
The clip walks the flow the dialog describes: right-click the map, pick "Set mission home", and the mission home appears where it was placed.
Recorded with refactor: keep the mission-planning home point local to the plan #2873 merged into master, so the menu carries both the renamed action and master's "Mission library" submenu, matching what will actually ship.
Reframed for an aquatic vehicle: the boat and its pier sit on one side, the mission path stays entirely over water, and the mission home lands at the pier beside the vehicle.
The upload step the dialog copy mentions is not in the clip, since its confirmation renders at the window's bottom-left, far outside a frame tight enough to keep the menu legible at the 260px the dialog renders it at.
Asset-only change:
HomePositionSettingHelp.vuealready consumes this file.Asset dropped from 1.9 MB to 415 KB.
To be merged after #2873
Closes #2943