Skip to content

Commit

Permalink
Send flight plan paths to the back of the map.
Browse files Browse the repository at this point in the history
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 dcs-liberation#3295.
  • Loading branch information
DanAlbert committed Dec 19, 2023
1 parent 80cb440 commit 1efce86
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 3 deletions.
1 change: 1 addition & 0 deletions changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
26 changes: 25 additions & 1 deletion client/src/components/flightplan/FlightPlan.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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}
/>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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} />, {
Expand Down

0 comments on commit 1efce86

Please sign in to comment.