From 0d224a663efd000be2979fc8bc890d2f9ea72c4c Mon Sep 17 00:00:00 2001 From: Kayla Firestack Date: Mon, 21 Oct 2024 08:41:02 -0400 Subject: [PATCH] feat(ts/components/markers): move vehicle marker into own file (#2859) * feat(ts/components/markers): move vehicle marker into own file * chore(ts/components/vehicleMarker): add imports * chore(ts/components/vehicleMarker): convert `Leaflet` reference to named import * feat(ts/components/vehicleMarker): move `vehicleMarker` css into own file --- assets/css/_vehicle_map.scss | 65 ------- assets/css/app.scss | 1 + assets/css/map/markers/_vehicle_marker.scss | 64 +++++++ assets/src/components/map.tsx | 2 +- .../components/map/markers/vehicleMarker.tsx | 162 ++++++++++++++++++ assets/src/components/mapMarkers.tsx | 159 +---------------- assets/src/components/mapPage/mapDisplay.tsx | 8 +- .../map/markers/vehicleMarker.test.tsx | 41 +++++ assets/tests/components/mapMarkers.test.tsx | 17 -- 9 files changed, 273 insertions(+), 246 deletions(-) create mode 100644 assets/css/map/markers/_vehicle_marker.scss create mode 100644 assets/src/components/map/markers/vehicleMarker.tsx create mode 100644 assets/tests/components/map/markers/vehicleMarker.test.tsx diff --git a/assets/css/_vehicle_map.scss b/assets/css/_vehicle_map.scss index fdfdf7763..cee2e67c0 100644 --- a/assets/css/_vehicle_map.scss +++ b/assets/css/_vehicle_map.scss @@ -15,71 +15,6 @@ $map-component-z-index: ( } } -.c-vehicle-map__label { - svg { - overflow: visible; - } - - &.primary { - font-size: var(--font-size-s); - font-weight: 500; - } - - &.secondary { - @include font-tiny; - } - - &.selected .c-vehicle-icon__label-background { - fill: $color-eggplant-300; - } -} - -.c-vehicle-map__icon { - path { - fill: $color-primary-legacy; - stroke: $color-bg-base; - stroke-width: 2; - - &.on-time { - fill: $color-vehicle-ontime; - } - - &.early.early-red { - fill: $color-vehicle-red; - } - &.early.early-blue { - fill: $color-vehicle-blue; - } - - &.late.early-red { - fill: $color-vehicle-blue; - } - &.late.early-blue { - fill: $color-vehicle-red; - } - - &.off-course { - fill: $color-vehicle-off-course; - } - - &.logged-out { - fill: $color-gray-300; - stroke: $color-gray-600; - stroke-width: 1px; - } - - &.selected { - stroke: $color-eggplant-600; - stroke-width: 1.5; - filter: drop-shadow(1px 1px 4px $color-eggplant-400); - } - } - - svg { - overflow: visible; - } -} - .c-vehicle-properties-panel__location .c-vehicle-map .leaflet-right { padding-right: calc(env(safe-area-inset-right) - #{$vpp-location-padding}); } diff --git a/assets/css/app.scss b/assets/css/app.scss index 3e2d2f841..3ad6dbded 100644 --- a/assets/css/app.scss +++ b/assets/css/app.scss @@ -73,6 +73,7 @@ $vpp-location-padding: 1rem; @import "map/markers/missed_stop_icon"; @import "map/markers/stop_icon"; @import "map/markers/user_location_marker"; +@import "map/markers/vehicle_marker"; @import "minimal_ladder_page"; @import "minischedule"; @import "modal"; diff --git a/assets/css/map/markers/_vehicle_marker.scss b/assets/css/map/markers/_vehicle_marker.scss new file mode 100644 index 000000000..596c81561 --- /dev/null +++ b/assets/css/map/markers/_vehicle_marker.scss @@ -0,0 +1,64 @@ +.c-vehicle-map__icon { + path { + fill: $color-primary-legacy; + stroke: $color-bg-base; + stroke-width: 2; + + &.on-time { + fill: $color-vehicle-ontime; + } + + &.early.early-red { + fill: $color-vehicle-red; + } + &.early.early-blue { + fill: $color-vehicle-blue; + } + + &.late.early-red { + fill: $color-vehicle-blue; + } + &.late.early-blue { + fill: $color-vehicle-red; + } + + &.off-course { + fill: $color-vehicle-off-course; + } + + &.logged-out { + fill: $color-gray-300; + stroke: $color-gray-600; + stroke-width: 1px; + } + + &.selected { + stroke: $color-eggplant-600; + stroke-width: 1.5; + filter: drop-shadow(1px 1px 4px $color-eggplant-400); + } + } + + svg { + overflow: visible; + } +} + +.c-vehicle-map__label { + svg { + overflow: visible; + } + + &.primary { + font-size: var(--font-size-s); + font-weight: 500; + } + + &.secondary { + @include font-tiny; + } + + &.selected .c-vehicle-icon__label-background { + fill: $color-eggplant-300; + } +} diff --git a/assets/src/components/map.tsx b/assets/src/components/map.tsx index e0f7708c0..af643e046 100644 --- a/assets/src/components/map.tsx +++ b/assets/src/components/map.tsx @@ -29,8 +29,8 @@ import { RouteStopMarkers, StationMarker, TrainVehicleMarker, - VehicleMarker, } from "./mapMarkers" +import { VehicleMarker } from "./map/markers/vehicleMarker" import ZoomLevelWrapper from "./ZoomLevelWrapper" import { StreetViewControl } from "./map/controls/StreetViewSwitch" import StreetViewModeEnabledContext from "../contexts/streetViewModeEnabledContext" diff --git a/assets/src/components/map/markers/vehicleMarker.tsx b/assets/src/components/map/markers/vehicleMarker.tsx new file mode 100644 index 000000000..9c48128a5 --- /dev/null +++ b/assets/src/components/map/markers/vehicleMarker.tsx @@ -0,0 +1,162 @@ +import { LatLngExpression, Marker } from "leaflet" +import React, { + PropsWithChildren, + useContext, + useRef, + useState, + useEffect, +} from "react" + +import { StateDispatchContext } from "../../../contexts/stateDispatchContext" +import { joinClasses } from "../../../helpers/dom" +import { vehicleLabel } from "../../../helpers/vehicleLabel" +import { statusClasses, drawnStatus } from "../../../models/vehicleStatus" + +import { Vehicle } from "../../../realtime" +import { ReactMarker } from "../utilities/reactMarker" + +interface VehicleMarkerProps extends PropsWithChildren { + vehicle: Vehicle + isPrimary: boolean + isSelected?: boolean + onSelect?: (vehicle: Vehicle) => void + shouldShowPopup?: boolean + onShouldShowPopupChange?: (newValue: boolean) => void +} + +export const VehicleMarker = ({ + children, + vehicle, + isPrimary, + onSelect, + isSelected = false, + shouldShowPopup = false, + onShouldShowPopupChange = () => {}, +}: VehicleMarkerProps) => { + const [{ userSettings }] = useContext(StateDispatchContext) + const markerRef = useRef>(null) + + const [isPopupVisible, setIsPopupVisible] = useState(false) + + useEffect(() => { + if (shouldShowPopup && !isPopupVisible) { + markerRef.current?.openPopup() + } + + if (!shouldShowPopup && isPopupVisible) { + markerRef.current?.closePopup() + } + }, [shouldShowPopup, isPopupVisible]) + + const eventHandlers = { + click: () => { + onSelect && onSelect(vehicle) + onShouldShowPopupChange(false) + }, + contextmenu: () => { + onShouldShowPopupChange(true) + }, + popupopen: () => { + setIsPopupVisible(true) + }, + popupclose: () => { + setIsPopupVisible(false) + onShouldShowPopupChange(false) + }, + } + const position: LatLngExpression = [vehicle.latitude, vehicle.longitude] + const labelBackgroundHeight = isPrimary ? 16 : 12 + const labelBackgroundWidth = + vehicleLabel(vehicle, userSettings).length <= 4 + ? isPrimary + ? 40 + : 30 + : isPrimary + ? 62 + : 40 + + // https://leafletjs.com/reference.html#marker-zindexoffset + // > By default, marker images zIndex is set automatically based on its latitude + // > [...] if you want to put the marker on top of all others, + // > [specify] a high value like 1000 [...] + const zIndexOffset = isSelected ? 1000 : 0 + + return ( + <> + + + + } + > + {children} + + + + + + {vehicleLabel(vehicle, userSettings)} + + + } + eventHandlers={eventHandlers} + zIndexOffset={zIndexOffset} + /> + + ) +} diff --git a/assets/src/components/mapMarkers.tsx b/assets/src/components/mapMarkers.tsx index ac720d3e4..324003818 100644 --- a/assets/src/components/mapMarkers.tsx +++ b/assets/src/components/mapMarkers.tsx @@ -1,20 +1,11 @@ import Leaflet, { LatLngExpression } from "leaflet" import "leaflet-defaulticon-compatibility" // see https://github.com/Leaflet/Leaflet/issues/4968#issuecomment-483402699 import "leaflet.fullscreen" -import React, { - PropsWithChildren, - useContext, - useEffect, - useRef, - useState, -} from "react" +import React, { useContext } from "react" import { Marker, Polyline, Popup, Tooltip } from "react-leaflet" -import { StateDispatchContext } from "../contexts/stateDispatchContext" import { joinClasses } from "../helpers/dom" -import { vehicleLabel } from "../helpers/vehicleLabel" -import { drawnStatus, statusClasses } from "../models/vehicleStatus" -import { TrainVehicle, Vehicle } from "../realtime" +import { TrainVehicle } from "../realtime" import { Shape, Stop } from "../schedule" import garages, { Garage as GarageData } from "../data/garages" @@ -36,152 +27,6 @@ import { fullStoryEvent } from "../helpers/fullStory" /* eslint-enable @typescript-eslint/ban-ts-comment */ -interface VehicleMarkerProps extends PropsWithChildren { - vehicle: Vehicle - isPrimary: boolean - isSelected?: boolean - onSelect?: (vehicle: Vehicle) => void - shouldShowPopup?: boolean - onShouldShowPopupChange?: (newValue: boolean) => void -} - -export const VehicleMarker = ({ - children, - vehicle, - isPrimary, - onSelect, - isSelected = false, - shouldShowPopup = false, - onShouldShowPopupChange = () => {}, -}: VehicleMarkerProps) => { - const [{ userSettings }] = useContext(StateDispatchContext) - const markerRef = useRef>(null) - - const [isPopupVisible, setIsPopupVisible] = useState(false) - - useEffect(() => { - if (shouldShowPopup && !isPopupVisible) { - markerRef.current?.openPopup() - } - - if (!shouldShowPopup && isPopupVisible) { - markerRef.current?.closePopup() - } - }, [shouldShowPopup, isPopupVisible]) - - const eventHandlers = { - click: () => { - onSelect && onSelect(vehicle) - onShouldShowPopupChange(false) - }, - contextmenu: () => { - onShouldShowPopupChange(true) - }, - popupopen: () => { - setIsPopupVisible(true) - }, - popupclose: () => { - setIsPopupVisible(false) - onShouldShowPopupChange(false) - }, - } - const position: LatLngExpression = [vehicle.latitude, vehicle.longitude] - const labelBackgroundHeight = isPrimary ? 16 : 12 - const labelBackgroundWidth = - vehicleLabel(vehicle, userSettings).length <= 4 - ? isPrimary - ? 40 - : 30 - : isPrimary - ? 62 - : 40 - - // https://leafletjs.com/reference.html#marker-zindexoffset - // > By default, marker images zIndex is set automatically based on its latitude - // > [...] if you want to put the marker on top of all others, - // > [specify] a high value like 1000 [...] - const zIndexOffset = isSelected ? 1000 : 0 - - return ( - <> - - - - } - > - {children} - - - - - - {vehicleLabel(vehicle, userSettings)} - - - } - eventHandlers={eventHandlers} - zIndexOffset={zIndexOffset} - /> - - ) -} - const makeTrainVehicleIcon = ({ bearing }: TrainVehicle): Leaflet.DivIcon => { const centerX = 24 const centerY = 24 diff --git a/assets/src/components/mapPage/mapDisplay.tsx b/assets/src/components/mapPage/mapDisplay.tsx index f5a7a1579..33ab349ae 100644 --- a/assets/src/components/mapPage/mapDisplay.tsx +++ b/assets/src/components/mapPage/mapDisplay.tsx @@ -36,12 +36,8 @@ import { drawerOffsetAutoCenter, InterruptibleFollower, } from "../map/follower" -import { - RouteShape, - RouteStopMarkers, - StopMarkers, - VehicleMarker, -} from "../mapMarkers" +import { RouteShape, RouteStopMarkers, StopMarkers } from "../mapMarkers" +import { VehicleMarker } from "../map/markers/vehicleMarker" import { MapSafeAreaContext } from "../../contexts/mapSafeAreaContext" import { LocationSearchMarker } from "../map/markers/locationSearchMarker" import ZoomLevelWrapper from "../ZoomLevelWrapper" diff --git a/assets/tests/components/map/markers/vehicleMarker.test.tsx b/assets/tests/components/map/markers/vehicleMarker.test.tsx new file mode 100644 index 000000000..d7ef16f80 --- /dev/null +++ b/assets/tests/components/map/markers/vehicleMarker.test.tsx @@ -0,0 +1,41 @@ +import { describe, test, expect } from "@jest/globals" + +import React, { PropsWithChildren } from "react" +import { render, screen } from "@testing-library/react" +import "@testing-library/jest-dom/jest-globals" + +import { VehicleMarker } from "../../../../src/components/map/markers/vehicleMarker" + +import { vehicleFactory } from "../../../factories/vehicle" +import { MapContainer } from "react-leaflet" + +const TestMap = ({ children }: PropsWithChildren): React.JSX.Element => ( + + {children} + +) + +describe("VehicleMarker", () => { + test("Includes icon and label", () => { + const { container } = render( + , + { + wrapper: TestMap, + } + ) + expect(container.querySelector(".c-vehicle-map__icon")).toBeInTheDocument() + expect(screen.getByText("101")).toBeInTheDocument() + }) +}) diff --git a/assets/tests/components/mapMarkers.test.tsx b/assets/tests/components/mapMarkers.test.tsx index 135f389af..acc5a0e8b 100644 --- a/assets/tests/components/mapMarkers.test.tsx +++ b/assets/tests/components/mapMarkers.test.tsx @@ -7,10 +7,8 @@ import { StationMarker, StopMarkers, TrainVehicleMarker, - VehicleMarker, } from "../../src/components/mapMarkers" import { stopFactory } from "../factories/stop" -import { vehicleFactory } from "../factories/vehicle" import { trainVehicleFactory } from "../factories/trainVehicle" import { render, screen } from "@testing-library/react" import React from "react" @@ -38,21 +36,6 @@ afterAll(() => { const stop = stopFactory.build() const station = stopFactory.build({ locationType: LocationType.Station }) -describe("VehicleMarker", () => { - test("Includes icon and label", () => { - const { container } = renderInMap( - - ) - expect(container.querySelector(".c-vehicle-map__icon")).toBeInTheDocument() - expect(screen.getByText("101")).toBeInTheDocument() - }) -}) - describe("TrainVehicleMarker", () => { test("Includes icon and label", () => { const { container } = renderInMap(