diff --git a/client/package-lock.json b/client/package-lock.json index 7ec0c6d1..49505ae0 100644 --- a/client/package-lock.json +++ b/client/package-lock.json @@ -41,7 +41,7 @@ "electron": "^21.1.0", "electron-is-dev": "^2.0.0", "generate-license-file": "^2.0.0", - "identity-obj-proxy": "^3.0.0", + "jest-transform-stub": "^2.0.0", "license-checker": "^25.0.1", "react-scripts": "5.0.1", "ts-node": "^10.9.1", @@ -12932,6 +12932,12 @@ "node": ">=8" } }, + "node_modules/jest-transform-stub": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/jest-transform-stub/-/jest-transform-stub-2.0.0.tgz", + "integrity": "sha512-lspHaCRx/mBbnm3h4uMMS3R5aZzMwyNpNIJLXj4cEsV0mIUtS4IjYJLSoyjRCtnxb6RIGJ4NL2quZzfIeNhbkg==", + "dev": true + }, "node_modules/jest-util": { "version": "27.5.1", "resolved": "https://registry.npmjs.org/jest-util/-/jest-util-27.5.1.tgz", @@ -30482,6 +30488,12 @@ } } }, + "jest-transform-stub": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/jest-transform-stub/-/jest-transform-stub-2.0.0.tgz", + "integrity": "sha512-lspHaCRx/mBbnm3h4uMMS3R5aZzMwyNpNIJLXj4cEsV0mIUtS4IjYJLSoyjRCtnxb6RIGJ4NL2quZzfIeNhbkg==", + "dev": true + }, "jest-util": { "version": "27.5.1", "resolved": "https://registry.npmjs.org/jest-util/-/jest-util-27.5.1.tgz", diff --git a/client/package.json b/client/package.json index 91e7c577..f71a3e02 100644 --- a/client/package.json +++ b/client/package.json @@ -69,7 +69,7 @@ "electron": "^21.1.0", "electron-is-dev": "^2.0.0", "generate-license-file": "^2.0.0", - "identity-obj-proxy": "^3.0.0", + "jest-transform-stub": "^2.0.0", "license-checker": "^25.0.1", "react-scripts": "5.0.1", "ts-node": "^10.9.1", @@ -80,7 +80,7 @@ "node_modules/(?!(@?react-leaflet|axios)/)" ], "moduleNameMapper": { - ".+\\.(css|styl|less|sass|scss|png|jpg|ttf|woff|woff2)$": "identity-obj-proxy" + ".+\\.(css|styl|less|sass|scss|png|jpg|ttf|woff|woff2)$": "jest-transform-stub" } } } diff --git a/client/src/components/waypointmarker/WaypointMarker.test.tsx b/client/src/components/waypointmarker/WaypointMarker.test.tsx new file mode 100644 index 00000000..f53e0a86 --- /dev/null +++ b/client/src/components/waypointmarker/WaypointMarker.test.tsx @@ -0,0 +1,134 @@ +import { renderWithProviders } from "../../testutils"; +import WaypointMarker, { TOOLTIP_ZOOM_LEVEL } from "./WaypointMarker"; +import { Map, Marker } from "leaflet"; +import React from "react"; +import { MapContainer } from "react-leaflet"; + +describe("WaypointMarker", () => { + it("is placed in the correct location", () => { + const waypoint = { + name: "", + position: { lat: 0, lng: 0 }, + altitude_ft: 0, + altitude_reference: "MSL", + is_movable: false, + should_mark: false, + include_in_path: true, + timing: "", + }; + const marker = React.createRef(); + renderWithProviders( + + + + ); + expect(marker.current?.getLatLng()).toEqual({ lat: 0, lng: 0 }); + }); + + it("tooltip is hidden when zoomed out", () => { + const waypoint = { + name: "", + position: { lat: 0, lng: 0 }, + altitude_ft: 0, + altitude_reference: "MSL", + is_movable: false, + should_mark: false, + include_in_path: true, + timing: "", + }; + const map = React.createRef(); + const marker = React.createRef(); + renderWithProviders( + + + + ); + map.current?.setView({ lat: 0, lng: 0 }, TOOLTIP_ZOOM_LEVEL - 1); + expect(marker.current?.getTooltip()?.isOpen()).toBeFalsy(); + }); + + it("tooltip is shown when zoomed in", () => { + const waypoint = { + name: "", + position: { lat: 0, lng: 0 }, + altitude_ft: 0, + altitude_reference: "MSL", + is_movable: false, + should_mark: false, + include_in_path: true, + timing: "", + }; + const map = React.createRef(); + const marker = React.createRef(); + renderWithProviders( + + + + ); + map.current?.setView({ lat: 0, lng: 0 }, TOOLTIP_ZOOM_LEVEL); + expect(marker.current?.getTooltip()?.isOpen()).toBeTruthy(); + }); + + it("tooltip has correct contents", () => { + const waypoint = { + name: "", + position: { lat: 0, lng: 0 }, + altitude_ft: 25000, + altitude_reference: "MSL", + is_movable: false, + should_mark: false, + include_in_path: true, + timing: "09:00:00", + }; + const map = React.createRef(); + const marker = React.createRef(); + renderWithProviders( + + + + ); + expect(marker.current?.getTooltip()?.getContent()).toEqual( + "0
25000 ft MSL
09:00:00" + ); + }); +}); diff --git a/client/src/components/waypointmarker/WaypointMarker.tsx b/client/src/components/waypointmarker/WaypointMarker.tsx index f279e290..8b33b757 100644 --- a/client/src/components/waypointmarker/WaypointMarker.tsx +++ b/client/src/components/waypointmarker/WaypointMarker.tsx @@ -3,13 +3,23 @@ import { Waypoint, useSetWaypointPositionMutation, } from "../../api/liberationApi"; +import mergeRefs from "../../mergeRefs"; import { Icon } from "leaflet"; import { Marker as LMarker } from "leaflet"; import icon from "leaflet/dist/images/marker-icon.png"; import iconShadow from "leaflet/dist/images/marker-shadow.png"; -import { MutableRefObject, useCallback, useEffect, useRef } from "react"; +import { + ForwardedRef, + MutableRefObject, + forwardRef, + useCallback, + useEffect, + useRef, +} from "react"; import { Marker, Tooltip, useMap, useMapEvent } from "react-leaflet"; +export const TOOLTIP_ZOOM_LEVEL = 9; + const WAYPOINT_ICON = new Icon({ iconUrl: icon, shadowUrl: iconShadow, @@ -22,84 +32,84 @@ interface WaypointMarkerProps { flight: Flight; } -const WaypointMarker = (props: WaypointMarkerProps) => { - // Most props of react-leaflet types are immutable and components will not - // update to account for changes, so we can't simply use the `permanent` - // property of the tooltip to control tooltip visibility based on the zoom - // level. - // - // On top of that, listening for zoom changes and opening/closing is not - // sufficient because clicking anywhere will close any opened tooltips (even - // if they are permanent; once openTooltip has been called that seems to no - // longer have any effect). - // - // Instead, listen for zoom changes and rebind the tooltip when the zoom level - // changes. - const map = useMap(); - const marker: MutableRefObject = useRef(); +const WaypointMarker = forwardRef( + (props: WaypointMarkerProps, ref: ForwardedRef) => { + // Most props of react-leaflet types are immutable and components will not + // update to account for changes, so we can't simply use the `permanent` + // property of the tooltip to control tooltip visibility based on the zoom + // level. + // + // On top of that, listening for zoom changes and opening/closing is not + // sufficient because clicking anywhere will close any opened tooltips (even + // if they are permanent; once openTooltip has been called that seems to no + // longer have any effect). + // + // Instead, listen for zoom changes and rebind the tooltip when the zoom level + // changes. + const map = useMap(); + const marker: MutableRefObject = useRef(null); - const [putDestination] = useSetWaypointPositionMutation(); + const [putDestination] = useSetWaypointPositionMutation(); - const rebindTooltip = useCallback(() => { - if (marker.current === undefined) { - return; - } + const rebindTooltip = useCallback(() => { + if (marker.current === null) { + return; + } - const tooltip = marker.current.getTooltip(); - if (tooltip === undefined) { - return; - } + const tooltip = marker.current.getTooltip(); + if (tooltip === undefined) { + return; + } - const permanent = map.getZoom() >= 9; - marker.current - .unbindTooltip() - .bindTooltip(tooltip, { permanent: permanent }); - }, [map]); - useMapEvent("zoomend", rebindTooltip); + const permanent = map.getZoom() >= TOOLTIP_ZOOM_LEVEL; + marker.current + .unbindTooltip() + .bindTooltip(tooltip, { permanent: permanent }); + }, [map]); + useMapEvent("zoomend", rebindTooltip); + + useEffect(() => { + const waypoint = props.waypoint; + marker.current?.setTooltipContent( + `${props.number} ${waypoint.name}
` + + `${waypoint.altitude_ft.toFixed()} ft ${ + waypoint.altitude_reference + }
` + + waypoint.timing + ); + }); - useEffect(() => { const waypoint = props.waypoint; - marker.current?.setTooltipContent( - `${props.number} ${waypoint.name}
` + - `${waypoint.altitude_ft.toFixed()} ft ${waypoint.altitude_reference}
` + - waypoint.timing + return ( + { + const m: LMarker = e.target; + m.setTooltipContent("Waiting to recompute TOT..."); + }, + dragend: async (e) => { + const m: LMarker = e.target; + const destination = m.getLatLng(); + try { + await putDestination({ + flightId: props.flight.id, + waypointIdx: props.number, + leafletPoint: { lat: destination.lat, lng: destination.lng }, + }); + } catch (e) { + console.error("Failed to set waypoint position", e); + } + }, + }} + ref={mergeRefs(ref, marker)} + > + + ); - }); - - const waypoint = props.waypoint; - return ( - { - const m: LMarker = e.target; - m.setTooltipContent("Waiting to recompute TOT..."); - }, - dragend: async (e) => { - const m: LMarker = e.target; - const destination = m.getLatLng(); - try { - await putDestination({ - flightId: props.flight.id, - waypointIdx: props.number, - leafletPoint: { lat: destination.lat, lng: destination.lng }, - }); - } catch (e) { - console.error("Failed to set waypoint position", e); - } - }, - }} - ref={(ref) => { - if (ref != null) { - marker.current = ref; - } - }} - > - - - ); -}; + } +); export default WaypointMarker; diff --git a/client/src/mergeRefs.test.tsx b/client/src/mergeRefs.test.tsx new file mode 100644 index 00000000..b95aa5be --- /dev/null +++ b/client/src/mergeRefs.test.tsx @@ -0,0 +1,17 @@ +import mergeRefs from "./mergeRefs"; + +describe("mergeRefs", () => { + it("merges all kinds of refs", () => { + const referent = "foobar"; + const ref = { current: null }; + var callbackResult = null; + const callbackRef = (node: string | null) => { + if (node != null) { + callbackResult = node; + } + }; + mergeRefs(ref, callbackRef)(referent); + expect(callbackResult).toEqual("foobar"); + expect(ref.current).toEqual("foobar"); + }); +}); diff --git a/client/src/mergeRefs.ts b/client/src/mergeRefs.ts new file mode 100644 index 00000000..a75fda76 --- /dev/null +++ b/client/src/mergeRefs.ts @@ -0,0 +1,16 @@ +import { ForwardedRef } from "react"; + +const mergeRefs = (...refs: ForwardedRef[]) => { + return (node: T) => { + for (const ref of refs) { + if (ref == null) { + } else if (typeof ref === "function") { + ref(node); + } else { + ref.current = node; + } + } + }; +}; + +export default mergeRefs;