Skip to content

Mission Planning: Recapture the mission-home help animation - #2995

Open
ArturoManzoli wants to merge 1 commit into
bluerobotics:masterfrom
ArturoManzoli:2943-recapture-mission-home-help
Open

Mission Planning: Recapture the mission-home help animation#2995
ArturoManzoli wants to merge 1 commit into
bluerobotics:masterfrom
ArturoManzoli:2943-recapture-mission-home-help

Conversation

@ArturoManzoli

@ArturoManzoli ArturoManzoli commented Sep 1, 2026

Copy link
Copy Markdown
Contributor
  • 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.vue already consumes this file.

  • Asset dropped from 1.9 MB to 415 KB.

Recaptured mission-home help animation

To be merged after #2873

Closes #2943

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
⚠️ IMPORTANT FIXES REQUIRED (Automated PR Review — round 1)

3 open findings: 1 major and 2 minor.

This PR swaps one file: the animated GIF shown inside the dialog that appears when someone tries to upload a mission without having placed a mission home. No code changes. The clip is meant to demonstrate the current right-click menu on the planning map; the menu wording it shows only exists once another pending PR lands, so the order in which the two merge decides whether the help matches the app.

What still needs attention

# 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 :340
  • src/views/MissionPlanningView.vue:278 and :341 — the checklist row and the shortcut pill
  • src/components/widgets/Map.vue:1488{ item: 'Set home waypoint', … }, and :1338 quotes 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 is Home 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:7class="rounded w-[260px] h-auto border-[#FFFFFF22] border-2 elevation-2"
  • :2 — the dialog is max-width="560px", and :5 pads the content with px-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.

@ArturoManzoli
ArturoManzoli marked this pull request as draft September 1, 2026 15:24
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.
@ArturoManzoli
ArturoManzoli force-pushed the 2943-recapture-mission-home-help branch from f3eca02 to 6842d08 Compare September 1, 2026 15:45
@ArturoManzoli
ArturoManzoli marked this pull request as ready for review September 1, 2026 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

mission-planning: recapture the mission-home help animation

1 participant