From 1efce862fb2cc3330a3cfaae372ffd6aefbf63a9 Mon Sep 17 00:00:00 2001
From: Dan Albert <dan@gingerhq.net>
Date: Mon, 18 Dec 2023 20:07:47 -0800
Subject: [PATCH] Send flight plan paths to the back of the map.

This fixes the unusual case where the `interactive: false` property had
no effect, which would make it impossible to plan missions against UI
elements that were overflown by many flights (such as the front line).

As an added bonus, it looks a bit nicer.

This impacts the test in an odd way, but the cure for that is probably
rewriting the test to not use a mock now that we've figured out how to
do that.

Fixes https://github.com/dcs-liberation/dcs_liberation/issues/3295.
---
 changelog.md                                  |  1 +
 .../src/components/flightplan/FlightPlan.tsx  | 26 ++++++++++++++++++-
 .../FlightPlansLayer.test.tsx                 |  8 ++++--
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/changelog.md b/changelog.md
index 215d3f98f..4afe28299 100644
--- a/changelog.md
+++ b/changelog.md
@@ -10,6 +10,7 @@ Saves from 9.x are not compatible with 10.0.0.
 
 * **[Flight Planning]** Aircraft from even numbered flights will no longer become inaccessible when canceling a draft package.
 * **[UI]** Flight members in the loadout menu are now numbered starting from 1 instead of 0.
+* **[UI]** Flight plan paths are now drawn behind all other map elements, fixing rare cases where they could prevent other UI elements from being clickable.
 
 # 9.0.0
 
diff --git a/client/src/components/flightplan/FlightPlan.tsx b/client/src/components/flightplan/FlightPlan.tsx
index 24d8fefc0..ff5e36d38 100644
--- a/client/src/components/flightplan/FlightPlan.tsx
+++ b/client/src/components/flightplan/FlightPlan.tsx
@@ -1,7 +1,8 @@
 import { Flight } from "../../api/liberationApi";
 import { useGetCommitBoundaryForFlightQuery } from "../../api/liberationApi";
 import WaypointMarker from "../waypointmarker";
-import { ReactElement } from "react";
+import { Polyline as LPolyline } from "leaflet";
+import { ReactElement, useEffect, useRef } from "react";
 import { Polyline } from "react-leaflet";
 
 const BLUE_PATH = "#0084ff";
@@ -27,16 +28,39 @@ const pathColor = (props: FlightPlanProps) => {
 function FlightPlanPath(props: FlightPlanProps) {
   const color = pathColor(props);
   const waypoints = props.flight.waypoints;
+
+  const polylineRef = useRef<LPolyline | null>(null);
+
+  // Flight paths should be drawn under everything else. There seems to be an
+  // issue where `interactive: false` doesn't do as its told (there's nuance,
+  // see the bug for details). It looks better if we draw the other elements on
+  // top of the flight plans anyway, so just push the flight plan to the back.
+  //
+  // https://github.com/dcs-liberation/dcs_liberation/issues/3295
+  //
+  // It's not possible to z-index a polyline (and leaflet says it never will be,
+  // because this is a limitation of SVG, not leaflet:
+  // https://github.com/Leaflet/Leaflet/issues/185), so we need to use
+  // bringToBack() to push the flight paths to the back of the drawing once
+  // they've been added to the map. They'll still draw on top of the map, but
+  // behind everything than was added before them. Anything added after always
+  // goes on top.
+  useEffect(() => {
+    polylineRef.current?.bringToBack();
+  });
+
   if (waypoints == null) {
     return <></>;
   }
   const points = waypoints
     .filter((waypoint) => waypoint.include_in_path)
     .map((waypoint) => waypoint.position);
+
   return (
     <Polyline
       positions={points}
       pathOptions={{ color: color, interactive: false }}
+      ref={polylineRef}
     />
   );
 }
diff --git a/client/src/components/flightplanslayer/FlightPlansLayer.test.tsx b/client/src/components/flightplanslayer/FlightPlansLayer.test.tsx
index 8b2f6985f..820957d0e 100644
--- a/client/src/components/flightplanslayer/FlightPlansLayer.test.tsx
+++ b/client/src/components/flightplanslayer/FlightPlansLayer.test.tsx
@@ -95,8 +95,12 @@ describe("FlightPlansLayer", () => {
           },
         },
       });
-      expect(mockPolyline).toHaveBeenCalledTimes(2);
-      expect(mockLayerGroup).toBeCalledTimes(1);
+
+      // For some reason passing ref to PolyLine causes it and its group to be
+      // redrawn, so these numbers don't match what you'd expect from the test.
+      // It probably needs to be rewritten without mocks.
+      expect(mockPolyline).toHaveBeenCalledTimes(3);
+      expect(mockLayerGroup).toBeCalledTimes(2);
     });
     it("are not drawn if wrong coalition", () => {
       renderWithProviders(<FlightPlansLayer blue={true} />, {