Skip to content

UI: Introducing flight planning - phase 1#4834

Open
blckmn wants to merge 19 commits intobetaflight:masterfrom
blckmn:flightplanning
Open

UI: Introducing flight planning - phase 1#4834
blckmn wants to merge 19 commits intobetaflight:masterfrom
blckmn:flightplanning

Conversation

@blckmn
Copy link
Member

@blckmn blckmn commented Jan 24, 2026

Available only with expert mode at the moment.

Phase 1 UI - Initial APP implementation for UX testing
Phase 2 FC - Implementation (cli configuration)
Phase 3 FC - APP integration (cli / msp)
Phase 4 FC - Flight testing

Summary by CodeRabbit

  • New Features

    • Flight Plan tab: full UI to create, edit, reorder, save, clear and persist flight plans with interactive map, elevation profile, waypoint list and editor.
  • Improvements

    • Map: draggable waypoint markers, click-to-add, live path updates, centering, responsive behavior, and more robust map cleanup.
    • Elevation: interactive elevation profile with stats, hover/tooltips and caching for elevation data.
    • Dialogs: conditional mounting, optional footer and smoother open/close behavior.
    • Options: Expert Mode toggle initialized from settings and persisted.
  • Localization

    • Added extensive English translations for Flight Plan UI and common actions.

Available only with expert mode at the moment.

Phase 1 UI - Initial APP implementation for UX testing.
Phase 2 FC - Implementation (cli configuration).
Phase 3 FC - APP integration (cli / msp)
Phase 4 FC - Flight testing
@blckmn blckmn self-assigned this Jan 24, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 24, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds a gated Flight Plan feature: new Vue flight_plan tab with map, elevation profile, waypoint list/editor, a useFlightPlan composable (state, persistence, CRUD, reorder), Dialog adjustments, expertMode wiring, map.destroy improvements, and many English localization entries.

Changes

Cohort / File(s) Summary
Localization
locales/en/messages.json
Added ~150+ flight-plan and generic UI translation keys, including HTML-markup messages.
Dialog component
src/components/elements/Dialog.vue
Render guard via v-if, footer slot, emit-driven open/close, async mount/watch, scroll reset, flex layout and footer styles.
Init & expertMode UI
src/components/init.js, src/components/tabs/OptionsTab.vue, src/index.html
expertMode initialized from ConfigStorage; OptionsTab exposes/persists expertMode toggle and syncs global state; index.html shows flight_plan tab conditionally.
Flight Plan composable
src/composables/useFlightPlan.js
New singleton-like composable: reactive waypoints, selection/editing UIDs, load/save via ConfigStorage, validation, CRUD, reorder, clear, i18n helpers, UI controls.
Map component
src/components/tabs/FlightPlan/FlightPlanMap.vue
New OpenLayers map: satellite base, magenta path, numbered markers, geolocation/fallback init, click-to-add, pointer drag-to-move waypoints, selection, centering, loading overlay, cleanup.
Elevation profile
src/components/tabs/FlightPlan/ElevationProfile.vue
New responsive SVG elevation chart, distance/altitude calculations, waypoint markers/tooltip, batched Open Elevation API sampling with per-segment caching and stale-response protection.
Waypoint UI
src/components/tabs/FlightPlan/WaypointList.vue, src/components/tabs/FlightPlan/WaypointEditor.vue
New list with drag-reorder, select/edit/delete (confirm); editor dialog with validation, conditional hold fields, add/update flows integrated with composable.
Flight Plan tab container
src/components/tabs/FlightPlanTab.vue
New tab composing map, elevation profile, waypoint list/editor, clear confirmation, bottom toolbar with Clear/Save, load/save wiring to composable.
Tab registration & routing
src/js/gui.js, src/js/main.js, src/js/vue_components.js
Register flight_plan in allowed tabs, add main switch-case for mounting Vue tab, and register FlightPlanTab in VueTabComponents and global app.
Misc UI / assets & utils
src/css/main.less, src/js/utils/map.js
Added route tab icon; improved map.destroy cleanup to clear features, remove/dispose layers and sources, and dispose map instance; minor header/tab markup updates.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Map as FlightPlanMap
    participant List as WaypointList
    participant Editor as WaypointEditor
    participant Composable as useFlightPlan
    participant Storage as ConfigStorage
    participant Profile as ElevationProfile

    User->>Map: click empty map (add)
    Map->>Composable: addWaypointAtLocation(lat,lng)
    Composable->>Composable: validateWaypoint()
    Composable->>Storage: setConfig(waypoints)
    Storage-->>Composable: saved
    Composable-->>Map: reactive waypoints update
    Composable-->>List: reactive waypoints update
    Map->>Map: render marker & path
    List->>User: show new waypoint in list

    User->>List: click edit
    List->>Composable: editWaypoint(uid)
    Composable-->>Editor: showEditorDialog = true
    Editor->>User: display form
    User->>Editor: change alt & save
    Editor->>Composable: updateWaypoint(uid, updates)
    Composable->>Storage: setConfig(waypoints)
    Storage-->>Composable: saved
    Composable-->>Map: waypoints updated
    Composable-->>Profile: waypoints changed

    Profile->>Profile: fetchGroundElevation() (batched external requests)
    Profile-->>User: render elevation & terrain
Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

Suggested labels

RN: UI, Vue

Suggested reviewers

  • haslinghuis
  • nerdCopter
  • sugaarK
🚥 Pre-merge checks | ✅ 2 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The description is vague and lacks required detail. While it mentions phase 1-4 plans, it provides minimal context about what the PR actually implements, the scope of changes, testing status, or known issues. Expand the description with details about implemented features, any known limitations or issues, testing performed, and which components were added/modified. Reference the template's guidance on clarity.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: introducing a flight planning UI feature as phase 1. It is concise, specific, and directly related to the primary changeset.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into master

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Fix all issues with AI agents
In `@src/components/tabs/FlightPlan/ElevationProfile.vue`:
- Around line 202-203: fetchGroundElevation can return out of order and
overwrite terrainSamples/groundElevation with stale data; add a monotonic ref
sequence token (e.g., elevationFetchSeq) that you increment each time before
starting an async fetch, capture the current token in a local variable inside
fetchGroundElevation, and after awaiting the fetch only apply updates to
terrainSamples and groundElevation if the captured token equals the latest
elevationFetchSeq; also ensure isFetchingElevation is set/cleared in the same
guarded path so only the latest response flips state. Apply the same
sequence-token pattern to the other fetch block in the same file (the similar
logic around lines 475-595).
- Around line 94-103: Replace the hard-coded profile label strings in the
ElevationProfile.vue template ("Avg Ground", "Alt", "Dist") with i18n calls
(e.g., $t('flightPlan.avgGround'), $t('flightPlan.alt'), $t('flightPlan.dist'))
wherever they appear (including the text nodes rendered in the SVG elements and
any other occurrences around the profile labels), and add corresponding keys to
the locale files; ensure you reference the component/template nodes (the <text>
elements that currently contain those literal strings) and update any tests or
snapshots if they assert exact string content.
- Around line 306-353: The y-axis and scaling ignore terrain heights because
scaleY and yAxisTicks use only maxAltitude; change the calculations to derive a
combined max (e.g., const combinedMax = Math.max(maxAltitude.value,
maxGroundElevation.value)) and use combinedMax instead of maxAltitude in both
yAxisTicks and scaleY (update range, paddedMax, and any default-range logic to
base on combinedMax) so terrain lines never render off-chart; reference
maxGroundElevation, maxAltitude, yAxisTicks, and scaleY when applying the
change.

In `@src/components/tabs/FlightPlan/WaypointEditor.vue`:
- Around line 142-174: The delayed reset in the watch of showEditorDialog can
fire after a quick reopen and wipe the form; introduce a module-scoped timeout
ID (e.g., closeResetTimeoutId) and, before scheduling setTimeout in the branch
that handles dialog close, clear any existing timeout via
clearTimeout(closeResetTimeoutId) so reopened dialogs won't have the pending
reset/cancelEdit applied; ensure to store the return value from setTimeout into
closeResetTimeoutId and also clear it when the timeout runs or when opening the
dialog (the watch branch that calls resetForm() for add mode).
- Around line 3-4: The form validation is being bypassed because the action
buttons are rendered outside the <form> (in the `#footer` slot) and the save
button is a type="button" that calls handleSave directly; update the component
so either the footer buttons are moved inside the form and the Save button is
changed to type="submit" (so native HTML5 validation runs and handleSave becomes
the form submit handler), or modify handleSave in WaypointEditor.vue to perform
validation first (e.g., call form.reportValidity() or explicitly validate
latitude/longitude/altitude fields before calling addWaypoint/updateWaypoint)
and abort if invalid; ensure references to addWaypoint/updateWaypoint and the
existing form element in WaypointEditor.vue are used so validation gates the
save logic.

In `@src/composables/useFlightPlan.js`:
- Around line 75-84: The current save routine rebuilds planData and sets both
currentPlan.createdAt and currentPlan.updatedAt to now, overwriting the original
creation time; update the logic that constructs planData (the object keyed by
STORAGE_KEY with currentPlan) to first read any existing stored plan (e.g.,
existing = storage[STORAGE_KEY]?.currentPlan or via the same persistence getter
used elsewhere) and set createdAt = existing?.createdAt ?? new
Date().toISOString(), while setting updatedAt = new Date().toISOString(); keep
the rest of currentPlan (name, waypoints from state.waypoints) unchanged and
write that planData back as before so createdAt is preserved on subsequent
saves.
🧹 Nitpick comments (5)
src/components/elements/Dialog.vue (2)

68-75: Consider using Vue's nextTick instead of setTimeout.

The current approach uses setTimeout(resolve, 0) to wait for DOM updates. Vue's nextTick is the idiomatic way to wait for the DOM to reflect reactive state changes.

♻️ Suggested refactor
+import { ref, watch, onMounted, nextTick } from "vue";
-import { ref, watch, onMounted } from "vue";
 watch(
     () => props.modelValue,
     async (newValue) => {
         if (newValue) {
             // Wait for next tick to ensure DOM is updated with v-if
-            await new Promise((resolve) => setTimeout(resolve, 0));
+            await nextTick();

             if (!dialogRef.value) {
                 return;
             }

96-105: Same suggestion: prefer nextTick over setTimeout in onMounted.

For consistency and idiomatic Vue code.

♻️ Suggested refactor
 onMounted(async () => {
     if (props.modelValue) {
         // Wait for next tick to ensure DOM is ready
-        await new Promise((resolve) => setTimeout(resolve, 0));
+        await nextTick();

         if (dialogRef.value && !dialogRef.value.open) {
             dialogRef.value.showModal();
         }
     }
 });
src/composables/useFlightPlan.js (1)

32-35: UID generation may produce collisions under rapid additions.

Date.now() + Math.random() could theoretically collide if two waypoints are added in the same millisecond. While unlikely in typical usage, consider using crypto.randomUUID() for guaranteed uniqueness.

♻️ Suggested refactor
     // Generate unique ID (timestamp + random for stability)
     const generateUid = () => {
-        return Date.now() + Math.random();
+        return crypto.randomUUID();
     };
src/components/tabs/FlightPlanTab.vue (1)

73-76: Remove debug logging before release.

console.log in the tab mount path can be noisy in production—consider removing or gating it once UX testing is complete.

src/components/tabs/FlightPlan/ElevationProfile.vue (1)

356-372: Avoid mutating profilePoints inside linePath.

Computed getters should be side‑effect‑free; mutating profilePoints couples marker positioning to linePath evaluation and makes the data flow fragile. Consider deriving a scaledProfilePoints computed and using it across paths/markers.

♻️ Suggested refactor
+const scaledProfilePoints = computed(() =>
+    profilePoints.value.map((point) => ({
+        ...point,
+        x: scaleX(point.distance),
+        y: scaleY(point.altitude),
+    })),
+);

 const linePath = computed(() => {
     if (profilePoints.value.length === 0) {
         return "";
     }
 
-    const points = profilePoints.value.map((point) => {
-        const x = scaleX(point.distance);
-        const y = scaleY(point.altitude);
-        return { x, y, ...point };
-    });
-
-    // Store scaled points for marker rendering
-    profilePoints.value.forEach((point, index) => {
-        point.x = points[index].x;
-        point.y = points[index].y;
-    });
+    const points = scaledProfilePoints.value;

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/components/tabs/FlightPlan/WaypointList.vue`:
- Around line 139-182: The drag handlers use event.target for styling which can
be inconsistent; update handleDragStart and handleDragEnd to use
event.currentTarget for adding/removing the "dragging" class (and any other DOM
manipulation that references the dragged element) so it always affects the
element the listener is attached to (refer to functions handleDragStart and
handleDragEnd and ensure the setTimeout in handleDragStart uses
event.currentTarget.classList.add and handleDragEnd uses
event.currentTarget.classList.remove); keep other logic (draggedUid,
dragOverUid, preventDefault, reorderWaypoints) unchanged.

In `@src/composables/useFlightPlan.js`:
- Around line 181-196: The reorderWaypoints function shifts indices when
removing an item then inserting it, causing an off-by-one when moving a waypoint
downward; fix by detecting when fromIndex < toIndex and decrementing toIndex by
1 after splicing out the movedWaypoint before inserting it back into
state.waypoints (keep using the existing reorderWaypoints, fromIndex, toIndex,
movedWaypoint identifiers), and ensure the function returns a success boolean
(true) after a successful reorder.
♻️ Duplicate comments (2)
src/composables/useFlightPlan.js (2)

73-82: createdAt is overwritten on every save (already flagged).
This matches a prior review note; keeping it here for visibility.


100-109: Replace || with ?? for numeric fields to allow valid zero values.

The validation accepts altitude >= 0, but altitude: waypointData.altitude || DEFAULT_ALTITUDE converts valid 0 to 400. Apply the same fix to latitude, longitude, and duration for consistency.

🛠️ Suggested fix
 const waypoint = {
     uid: generateUid(),
-    latitude: waypointData.latitude || 0,
-    longitude: waypointData.longitude || 0,
-    altitude: waypointData.altitude || DEFAULT_ALTITUDE,
+    latitude: waypointData.latitude ?? 0,
+    longitude: waypointData.longitude ?? 0,
+    altitude: waypointData.altitude ?? DEFAULT_ALTITUDE,
     type: waypointData.type || DEFAULT_TYPE,
-    duration: waypointData.duration || 0,
-    pattern: waypointData.pattern || "circle",
+    duration: waypointData.duration ?? 0,
+    pattern: waypointData.pattern ?? "circle",
     order: state.waypoints.length,
 };

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/components/tabs/FlightPlan/ElevationProfile.vue`:
- Around line 227-251: interpolatePoint currently divides by Math.sin(d) where d
is zero for identical waypoints, producing NaN; add a guard at the start of
interpolatePoint (using calculateDistance or the computed angular d) to detect
zero/near-zero distance and return the start coordinates immediately (return
{latitude: lat1, longitude: lon1}) to avoid the division-by-zero; update the
function referenced as interpolatePoint so downstream terrain sampling no longer
receives NaN.

In `@src/components/tabs/FlightPlan/WaypointList.vue`:
- Around line 139-148: In handleDragStart, event.currentTarget becomes null
inside the setTimeout so capture the element synchronously (e.g., const el =
event.currentTarget) and use that reference inside the timeout (with a null
check) instead of event.currentTarget; update the function around the draggedUid
assignment and the setTimeout so it references the captured element and safely
calls el.classList.add("dragging").
🧹 Nitpick comments (4)
src/composables/useFlightPlan.js (1)

104-113: Falsy coordinate check silently replaces valid zero values.

Lines 106-107 use waypointData.latitude || 0 and waypointData.longitude || 0. If a user explicitly provides latitude: 0 or longitude: 0 (valid coordinates on the equator/prime meridian), these are falsy in JavaScript and will be replaced with 0 anyway—which happens to work here. However, if the default were ever changed, this pattern would cause bugs.

Additionally, after validateWaypoint already confirmed the coordinates are valid, this fallback is redundant. Consider using nullish coalescing for consistency and clarity.

♻️ Suggested improvement
     const waypoint = {
         uid: generateUid(),
-        latitude: waypointData.latitude || 0,
-        longitude: waypointData.longitude || 0,
-        altitude: waypointData.altitude || DEFAULT_ALTITUDE,
+        latitude: waypointData.latitude ?? 0,
+        longitude: waypointData.longitude ?? 0,
+        altitude: waypointData.altitude ?? DEFAULT_ALTITUDE,
         type: waypointData.type || DEFAULT_TYPE,
         duration: waypointData.duration || 0,
         pattern: waypointData.pattern || "circle",
         order: state.waypoints.length,
     };
src/components/tabs/FlightPlan/WaypointList.vue (1)

179-183: event.currentTarget may be unreliable in handleDragEnd depending on timing.

While handleDragEnd doesn't use setTimeout, the same principle applies—if this handler is invoked in unusual circumstances where the event has been recycled, currentTarget could be stale. For consistency with the fix in handleDragStart, consider capturing the element synchronously at the start of the function.

Alternatively, since dragend fires synchronously on the dragged element, event.currentTarget should be valid here. You may keep this as-is if testing confirms it works reliably.

src/components/tabs/FlightPlan/ElevationProfile.vue (2)

361-385: Computed property has side effects—mutating profilePoints objects.

Lines 374-377 mutate objects within profilePoints.value (a computed property) by assigning x and y properties. Computed properties should be pure and side-effect-free. This can cause unexpected reactivity issues and makes the code harder to reason about.

Consider returning the scaled coordinates as part of the computed result or using a separate reactive state for scaled points.

♻️ Suggested approach
 const linePath = computed(() => {
     if (profilePoints.value.length === 0) {
         return "";
     }

     const points = profilePoints.value.map((point) => {
         const x = scaleX(point.distance);
         const y = scaleY(point.altitude);
         return { x, y, ...point };
     });

-    // Store scaled points for marker rendering
-    profilePoints.value.forEach((point, index) => {
-        point.x = points[index].x;
-        point.y = points[index].y;
-    });
-
     const pathParts = points.map((point, index) => {
         const command = index === 0 ? "M" : "L";
         return `${command} ${point.x} ${point.y}`;
     });

     return pathParts.join(" ");
 });
+
+// Separate computed for scaled profile points (used by markers)
+const scaledProfilePoints = computed(() => {
+    return profilePoints.value.map((point) => ({
+        ...point,
+        x: scaleX(point.distance),
+        y: scaleY(point.altitude),
+    }));
+});

Then update the template to use scaledProfilePoints for the waypoint markers instead of profilePoints.


559-582: Consider adding a timeout to the external API calls.

The fetch to api.open-elevation.com has no timeout configured. If the API is slow or unresponsive, the UI could hang indefinitely in the loading state. For a Phase 1 UX testing feature this is acceptable, but consider adding AbortController with a timeout for production use.

@sugaarK
Copy link
Member

sugaarK commented Jan 24, 2026

very cool one thing when you zoom the map and them move a waypoint. when you let the waypoint go it snaps the zoom back out to std zoom

@haslinghuis haslinghuis moved this to App in 2026.6.0 Jan 24, 2026
@haslinghuis haslinghuis added this to the 2026.6 milestone Jan 24, 2026
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In `@src/components/tabs/FlightPlan/ElevationProfile.vue`:
- Around line 486-499: The early-return in fetchGroundElevation allows in-flight
responses to overwrite state because elevationFetchSeq is only incremented after
the empty-path check; move the elevationFetchSeq.value++ (and capture
currentSeq) to before the waypoints.value.length === 0 check so any older
responses are invalidated, and when returning for an empty path ensure
isFetchingElevation.value is set to false and groundElevation.value and
terrainSamples.value are reset as currently intended; update references to
elevationFetchSeq, currentSeq, isFetchingElevation, groundElevation,
terrainSamples, and waypoints in fetchGroundElevation accordingly.

In `@src/components/tabs/FlightPlan/FlightPlanMap.vue`:
- Around line 205-211: The comment and threshold are wrong because movedDistance
is computed in projected map units (meters) using dragStartCoordinate and
event.coordinate, so either update the comment to state "map units/meters" and
increase the threshold to an appropriate meter value, or switch to a pixel-based
check by converting coordinates with
mapInstance.value.map.getPixelFromCoordinate (use the pixel positions and
compute pixelDistance) and use a small pixel threshold (e.g. ~3 pixels) before
treating movement as a drag; locate the calculation around movedDistance,
dragStartCoordinate, and event.coordinate in FlightPlanMap.vue and replace the
distance/threshold logic accordingly.
🧹 Nitpick comments (3)
src/components/tabs/FlightPlan/FlightPlanMap.vue (3)

48-115: Consider extracting duplicated map initialization logic.

The initMap call with nearly identical configuration appears three times. Extracting this to a helper would reduce duplication and make future changes easier.

♻️ Suggested refactor
 onMounted(() => {
     if (!mapRef.value) {
         console.error("Map ref not available");
         return;
     }

-    // Get user's location and initialize map
-    // Zoom level 15 shows approximately 1 nautical mile (1852m) in view
     const defaultZoom = 15;
     const fallbackLat = 0;
     const fallbackLon = 0;

+    const initializeMap = (lat, lon) => {
+        mapInstance.value = initMap({
+            target: mapRef.value,
+            defaultZoom,
+            defaultLat: lat,
+            defaultLon: lon,
+            defaultLayer: "satellite",
+        });
+        setupMapLayers();
+    };
+
     // Try to get user's geolocation
     if (navigator.geolocation) {
         navigator.geolocation.getCurrentPosition(
             (position) => {
                 const { latitude, longitude } = position.coords;
                 console.log("User location obtained:", latitude, longitude);
-
-                // Initialize map at user's location
-                mapInstance.value = initMap({
-                    target: mapRef.value,
-                    defaultZoom: defaultZoom,
-                    defaultLat: latitude,
-                    defaultLon: longitude,
-                    defaultLayer: "satellite",
-                });
-
-                console.log("Map initialized at user location");
-                setupMapLayers();
+                initializeMap(latitude, longitude);
             },
             (error) => {
                 console.warn("Geolocation failed, using fallback:", error.message);
-
-                // Initialize map at fallback location
-                mapInstance.value = initMap({
-                    target: mapRef.value,
-                    defaultZoom: defaultZoom,
-                    defaultLat: fallbackLat,
-                    defaultLon: fallbackLon,
-                    defaultLayer: "satellite",
-                });
-
-                console.log("Map initialized at fallback location");
-                setupMapLayers();
+                initializeMap(fallbackLat, fallbackLon);
             },
             {
                 enableHighAccuracy: true,
                 timeout: 5000,
                 maximumAge: 0,
             },
         );
     } else {
         console.warn("Geolocation not supported, using fallback");
-
-        // Initialize map at fallback location
-        mapInstance.value = initMap({
-            target: mapRef.value,
-            defaultZoom: defaultZoom,
-            defaultLat: fallbackLat,
-            defaultLon: fallbackLon,
-            defaultLayer: "satellite",
-        });
-
-        console.log("Map initialized at fallback location");
-        setupMapLayers();
+        initializeMap(fallbackLat, fallbackLon);
     }
 });

246-250: Redundant drag check in click handler.

The isDragging.value check at line 248 is effectively dead code since pointerup always fires before click and resets isDragging to false. The waypoint addition logic still works correctly due to the waypointClicked check, but this guard can be removed for clarity.

♻️ Suggested cleanup
     // Click handler - add new waypoint when clicking on empty map
     mapInstance.value.map.on("click", (event) => {
-        // Don't add waypoint if we were dragging
-        if (isDragging.value) {
-            return;
-        }
-
         // Check if a waypoint marker was clicked
         const waypointClicked = mapInstance.value.map.hasFeatureAtPixel(event.pixel, {
             layerFilter: (layer) => layer === waypointLayer.value,
         });

343-346: Variable shadowing: local isDragging shadows the outer ref.

The local const isDragging shadows the outer isDragging ref, which could cause confusion or bugs if the ref is accessed later in this scope. Rename the local variable for clarity.

♻️ Suggested fix
         // Check if this waypoint is selected or being dragged
         const isSelected = selectedWaypointUid.value === wp.uid;
-        const isDragging = draggingWaypointUid.value === wp.uid;
+        const isBeingDragged = draggingWaypointUid.value === wp.uid;

         // Determine color: green for dragging, orange for selected, blue for normal
         let fillColor = "#0080FF"; // Blue for normal
         let radius = 10;
         let strokeWidth = 2;
         let fontSize = "bold 12px sans-serif";

-        if (isDragging) {
+        if (isBeingDragged) {
             fillColor = "#00FF00"; // Green for dragging

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/components/tabs/FlightPlan/ElevationProfile.vue`:
- Around line 523-551: The loop that generates terrain samples can emit a
duplicate sample at a segment endpoint when segmentDistanceNM is an exact
multiple of TERRAIN_SAMPLE_INTERVAL_NM; update the sampling loop in the block
that uses numSamples, j, fraction, interpolatePoint, and samples so it skips
creating a sample when fraction >= 1 (or when j === numSamples and
segmentDistanceNM % TERRAIN_SAMPLE_INTERVAL_NM === 0), ensuring only one
endpoint sample is added by the later "Add current waypoint" push and thereby
preventing duplicate endpoint entries and skewed distances.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@src/components/tabs/FlightPlan/FlightPlanMap.vue`:
- Around line 436-442: The destroy implementation created in initMap currently
only calls map.setTarget(null) and vectorSource.clear() and must fully dispose
OpenLayers resources to avoid leaks; update the destroy function used by
FlightPlanMap.vue to either call map.dispose() directly (preferred) or else
iterate and remove all layers and dispose each layer's source, clear
vectorSource, unbind event listeners/interactions, and dispose the View
(replacing the single map.setTarget(null) in the destroy implementation);
reference the destroy function in initMap, map.dispose(), map.setTarget(null),
and vectorSource.clear() when making the change.
🧹 Nitpick comments (2)
src/components/tabs/FlightPlan/FlightPlanMap.vue (2)

62-94: Potential race condition if component unmounts during geolocation request.

The geolocation callback could fire after the component unmounts, attempting to initialize the map on a stale or null mapRef. Consider adding a guard using a mounted flag.

♻️ Suggested fix
+const isMounted = ref(true);
+
 // Initialize map and layers
 onMounted(() => {
     if (!mapRef.value) {
         console.error("Map ref not available");
         return;
     }
 
     // Get user's location and initialize map
     const fallbackLat = 0;
     const fallbackLon = 0;
 
     // Try to get user's geolocation
     if (navigator.geolocation) {
         navigator.geolocation.getCurrentPosition(
             (position) => {
+                if (!isMounted.value) return;
                 const { latitude, longitude } = position.coords;
                 console.log("User location obtained:", latitude, longitude);
                 initializeMapAtLocation(latitude, longitude, "Map initialized at user location");
             },
             (error) => {
+                if (!isMounted.value) return;
                 console.warn("Geolocation failed, using fallback:", error.message);
                 initializeMapAtLocation(fallbackLat, fallbackLon, "Map initialized at fallback location");
             },
             // ...
         );
     }
     // ...
 });
 
 // Cleanup on unmount
 onUnmounted(() => {
+    isMounted.value = false;
     console.log("Cleaning up map");
     // ...
 });

151-151: Consider removing or guarding console.log statements for production.

There are numerous console.log statements throughout the component (lines 57, 77, 151, 197, 206, 238, 303, 372, 383, 431, 438). While useful during Phase 1 UX testing, consider wrapping them in a debug flag or removing them before production release.

@haslinghuis
Copy link
Member

haslinghuis commented Jan 24, 2026

Reset browser settings

image

Have to zoom out before map appears - and then can zoom in.

Network location provider at 'https://www.googleapis.com/' : Returned error code 400.

https://discuss.ai.google.dev/t/generate-object-error-user-location-is-not-supported-for-the-api-use/37440

…o Flight Time.

Also added Max terrain elevation to elevation profile.
Defaulting to the IP address location instead of 0,0, with a final as the Sydney Harbour Bridge.
Adjusted the max number of elevation requests, and adjusted to use POST to minimise API usage.
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Fix all issues with AI agents
In `@src/components/tabs/FlightPlan/ElevationProfile.vue`:
- Around line 357-387: The totalFlightTime computed uses wp.speed when computing
segmentTime = distanceNM / speed which can divide by zero and produce Infinity,
and minutes rounding can produce "1:60"; update the loop in totalFlightTime
(referencing totalFlightTime, waypoints, calculateDistance,
METERS_TO_NAUTICAL_MILES) to guard against zero/negative speed by treating speed
<= 0 as a small minimum (or skip the segment) before division, and after
computing hours and minutes add a rollover check that if minutes === 60 then
increment hours by 1 and set minutes to 0 so the formatted string never shows
":60".

In `@src/components/tabs/FlightPlan/FlightPlanMap.vue`:
- Around line 61-76: The fetchIPLocation function currently uses an HTTP-only
endpoint; replace "http://ip-api.com/json" with an HTTPS-compatible provider
(for example "https://get.geojs.io/v1/ip/geo.json" or "https://ipapi.co/json/")
and update the response parsing in fetchIPLocation to handle that provider's
fields (e.g., geojs returns string "latitude"/"longitude" so coerce to Number,
ipapi returns lat/lon numbers). Ensure the fetch URL is HTTPS, keep the existing
error handling, and return { latitude, longitude } or null as currently
implemented.

In `@src/composables/useFlightPlan.js`:
- Around line 105-113: The waypoint object construction uses logical OR (||)
which overrides valid zero values; update the properties in the waypoint literal
created where uid is set via generateUid()—specifically latitude, longitude,
altitude, speed, type, duration, and pattern—to use nullish coalescing (??)
instead of || so that only null or undefined trigger defaults like
DEFAULT_ALTITUDE, DEFAULT_SPEED, DEFAULT_TYPE, and the pattern default remains
"circle".
♻️ Duplicate comments (1)
src/components/tabs/FlightPlan/WaypointEditor.vue (1)

185-208: Clear the pending close-reset when reopening in edit mode.

Line 188 clears closeResetTimeoutId only in add mode; reopening quickly in edit mode can still let the delayed reset fire and wipe the form/cancel the edit.

🛠️ Proposed fix
 watch(
     () => showEditorDialog.value,
     (isOpen, wasOpen) => {
+        if (isOpen && closeResetTimeoutId !== null) {
+            clearTimeout(closeResetTimeoutId);
+            closeResetTimeoutId = null;
+        }
         if (isOpen && !editingWaypointUid.value) {
             // Dialog opened in add mode, clear any pending reset and reset form to defaults
-            if (closeResetTimeoutId !== null) {
-                clearTimeout(closeResetTimeoutId);
-                closeResetTimeoutId = null;
-            }
             resetForm();
         } else if (!isOpen && wasOpen) {

Copy link
Member

@haslinghuis haslinghuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This time google api did not complain with 400 - as it did a few hours ago 😏

Image

Anyway no longer need to do the zoom trick.

@mituritsyn
Copy link
Contributor

The tab disappears if you connect and disconnect from fc.
image

@mituritsyn
Copy link
Contributor

Can the map form with controls from the GPS tab be reused?
image

@blckmn
Copy link
Member Author

blckmn commented Jan 26, 2026

The tab disappears if you connect and disconnect from fc.

image

At the minute it is just UX and isn't designed to work with an FC. Once the FC code is finished, it'll remain available when connected.

@blckmn
Copy link
Member Author

blckmn commented Jan 26, 2026

This time google api did not complain with 400 - as it did a few hours ago 😏

Image

Anyway no longer need to do the zoom trick.

If it can't get location from the browser it'll attempt to use your IP for an approximate, otherwise you'll get a nice Australian landmark ;)

# Conflicts:
#	src/js/vue_components.js
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@locales/en/messages.json`:
- Around line 3219-3356: The Flight Plan messages and logic currently hardcode
aviation units (see message keys flightPlanAltitude, flightPlanSpeed,
flightPlanDuration and the altitude/speed ranges 0–50000 ft / 0–500 knots plus
defaults 400 ft / 10 knots), but the app already has OSD unit prefs
(IMPERIAL/METRIC/BRITISH in src/js/tabs/osd.js); update the Flight Plan
composable (where waypoint validation/calculation/defaults live) to read the
global unit preference and (a) convert and display units in labels
(flightPlanAltitude, flightPlanSpeed, flightPlanDuration), (b) convert
validation ranges/default values and distance/speed/time calculations
accordingly, and (c) ensure stored waypoint values remain canonical or clearly
documented; alternatively, if aviation-only units are intentional, add a brief
developer/user-facing note in the Flight Plan composable and i18n file
explaining that Flight Plan intentionally uses aviation units and will not
follow OSD prefs.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarQube Cloud

@github-actions
Copy link
Contributor

🎉 Do you want to test this code? 🎉

⚠️ CAUTION: The build may be unstable and result in corrupted configurations or data loss. Use only for testing! ⚠️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: App

Development

Successfully merging this pull request may close these issues.

4 participants