From 374759df0f45c1ab8fcd26037874e85fbaf386c6 Mon Sep 17 00:00:00 2001 From: Dan Albert Date: Tue, 27 Jun 2023 22:24:37 -0700 Subject: [PATCH] Test most of WaypointMarker. Unlike the other map tests which heavily rely on mocks, this one uses React refs to inspect the constructed leaflet objects. The DOM itself doesn't appear to contain anything worth testing against (react-leaflet rendering doesn't work like typical React rendering). This required some infrastructure changes: 1. Forwarded ref from WaypointMarker to Marker so the test can observe it. Added a mergeRefs helper (and its own tests) to make that easier. 2. Switched from identity-obj-proxy to jest-transform-stub, because the former doesn't produce a useable image for imports, and we need usable images for leaflet to be able to render. This doesn't yet test drag and drop behavior since that requires mocking the backend, and this commit is already complicated enough. That'll be next. --- client/package-lock.json | 14 +- client/package.json | 4 +- .../waypointmarker/WaypointMarker.test.tsx | 134 +++++++++++++++ .../waypointmarker/WaypointMarker.tsx | 156 ++++++++++-------- client/src/mergeRefs.test.tsx | 17 ++ client/src/mergeRefs.ts | 16 ++ 6 files changed, 265 insertions(+), 76 deletions(-) create mode 100644 client/src/components/waypointmarker/WaypointMarker.test.tsx create mode 100644 client/src/mergeRefs.test.tsx create mode 100644 client/src/mergeRefs.ts 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;