From c07133872bc4c398c619996bb7943280c2f462df Mon Sep 17 00:00:00 2001 From: Josh Larson Date: Tue, 6 Feb 2024 10:31:37 -0500 Subject: [PATCH] feat: Add dropdown when right-clicking on a vehicle marker (#2373) * feat: Add dropdown when right-clicking on a vehicle marker * refactor: Extract to its own file * feat(storybook): Add Storybook story for * feat(test-groups): Gate "Start Detour" dropdown behind a test group * feat: Only allow the dropdown on desktop and tablet * refactor: Make the popup a children property of the * refactor: Make shouldShowPopup state that's passed into a * feat: Close popup when its menu item is clicked * test: Move the dropdown tests so that they test the right use case * test: Assert using getByRole and queryByRole rather than looking for CSS classes --- assets/css/_dropdown.scss | 8 ++ assets/css/app.scss | 1 + assets/css/bootstrap.scss | 2 +- assets/src/components/map/dropdown.tsx | 17 +++ assets/src/components/mapMarkers.tsx | 65 +++++++-- assets/src/components/mapPage/mapDisplay.tsx | 43 +++++- assets/src/userInTestGroup.ts | 1 + .../skate-components/map/dropdown.stories.tsx | 43 ++++++ assets/tests/components/map.test.tsx | 6 +- .../components/mapPage/mapDisplay.test.tsx | 136 +++++++++++++++++- 10 files changed, 307 insertions(+), 15 deletions(-) create mode 100644 assets/css/_dropdown.scss create mode 100644 assets/src/components/map/dropdown.tsx create mode 100644 assets/stories/skate-components/map/dropdown.stories.tsx diff --git a/assets/css/_dropdown.scss b/assets/css/_dropdown.scss new file mode 100644 index 000000000..cf2b26692 --- /dev/null +++ b/assets/css/_dropdown.scss @@ -0,0 +1,8 @@ +.c-dropdown-popup-wrapper { + visibility: hidden; +} + +.c-dropdown-popup-menu { + position: static; + visibility: visible; +} diff --git a/assets/css/app.scss b/assets/css/app.scss index e5d6bf464..a4e30e040 100644 --- a/assets/css/app.scss +++ b/assets/css/app.scss @@ -52,6 +52,7 @@ $z-properties-panel-context: ( @import "disconnected_modal"; @import "diversion_page"; @import "drawer_tab"; +@import "dropdown"; @import "filter_accordion"; @import "garage_filter"; @import "icon_alert_circle"; diff --git a/assets/css/bootstrap.scss b/assets/css/bootstrap.scss index 1275e9fea..021ff75be 100644 --- a/assets/css/bootstrap.scss +++ b/assets/css/bootstrap.scss @@ -33,7 +33,7 @@ // @import "../node_modules/bootstrap/scss/carousel"; // @import "../node_modules/bootstrap/scss/close"; // @import "../node_modules/bootstrap/scss/containers"; -// @import "../node_modules/bootstrap/scss/dropdown"; +@import "../node_modules/bootstrap/scss/dropdown"; @import "../node_modules/bootstrap/scss/forms"; // @import "../node_modules/bootstrap/scss/grid"; // @import "../node_modules/bootstrap/scss/images"; diff --git a/assets/src/components/map/dropdown.tsx b/assets/src/components/map/dropdown.tsx new file mode 100644 index 000000000..757041f92 --- /dev/null +++ b/assets/src/components/map/dropdown.tsx @@ -0,0 +1,17 @@ +import React, { PropsWithChildren } from "react" +import { Dropdown } from "react-bootstrap" + +interface DropdownMenuProps extends PropsWithChildren {} + +export const DropdownMenu = ({ children }: DropdownMenuProps) => { + return ( + + {children} + + ) +} + +export const DropdownItem = Dropdown.Item diff --git a/assets/src/components/mapMarkers.tsx b/assets/src/components/mapMarkers.tsx index 400eedbcb..e08dd899c 100644 --- a/assets/src/components/mapMarkers.tsx +++ b/assets/src/components/mapMarkers.tsx @@ -1,7 +1,13 @@ import Leaflet, { LatLngExpression } from "leaflet" import "leaflet-defaulticon-compatibility" // see https://github.com/Leaflet/Leaflet/issues/4968#issuecomment-483402699 import "leaflet.fullscreen" -import React, { useContext } from "react" +import React, { + PropsWithChildren, + useContext, + useEffect, + useRef, + useState, +} from "react" import { Marker, Polyline, Popup, Tooltip } from "react-leaflet" import { StateDispatchContext } from "../contexts/stateDispatchContext" @@ -96,19 +102,55 @@ const makeLabelIcon = ( }) } +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, -}: { - vehicle: Vehicle - isPrimary: boolean - isSelected?: boolean - onSelect?: (vehicle: Vehicle) => void -}) => { + shouldShowPopup = false, + onShouldShowPopupChange = () => {}, +}: VehicleMarkerProps) => { const [{ userSettings }] = useContext(StateDispatchContext) - const eventHandlers = onSelect ? { click: () => onSelect(vehicle) } : {} + 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 vehicleIcon: Leaflet.DivIcon = makeVehicleIcon( vehicle, @@ -128,6 +170,7 @@ export const VehicleMarker = ({ // > [...] if you want to put the marker on top of all others, // > [specify] a high value like 1000 [...] const zIndexOffset = isSelected ? 1000 : 0 + return ( <> + ref={markerRef} + > + {children} + + (false) + return ( <> {selectedVehicleOrGhost && ( @@ -292,7 +310,26 @@ const SelectedVehicleDataLayers = ({ isPrimary={true} isSelected={true} onSelect={selectVehicle} - /> + shouldShowPopup={shouldShowPopup} + onShouldShowPopupChange={setShouldShowPopup} + > + {dropdownEnabled && ( + + + { + setShouldShowPopup(false) + }} + > + Start a detour on route {selectedVehicleOrGhost.routeId} + + + + )} + {!selectedVehicleOrGhost.isShuttle && ( { + const children = ctx.args["children"] as Array + + ctx.args["children"] = children.map((child) => ( + {child} + )) + + return + }, + ], +} satisfies Meta + +export default meta +type Story = StoryObj + +export const Default: Story = {} + +export const WithMoreEntries: Story = { + args: { + children: [ + "Start a detour on route 66", + "Hold this bus for 10 minutes", + "View old detours for this route", + ], + }, +} diff --git a/assets/tests/components/map.test.tsx b/assets/tests/components/map.test.tsx index a52660042..9494d4181 100644 --- a/assets/tests/components/map.test.tsx +++ b/assets/tests/components/map.test.tsx @@ -30,7 +30,7 @@ import { runIdToLabel } from "../../src/helpers/vehicleLabel" import getTestGroups from "../../src/userTestGroups" import { LocationType } from "../../src/models/stopData" import { setHtmlDefaultWidthHeight } from "../testHelpers/leafletMapWidth" -import { mockTileUrls } from "../testHelpers/mockHelpers" +import { mockScreenSize, mockTileUrls } from "../testHelpers/mockHelpers" import { streetViewModeSwitch } from "../testHelpers/selectors/components/mapPage/map" import { streetViewUrl } from "../../src/util/streetViewUrl" import shapeFactory from "../factories/shape" @@ -78,6 +78,10 @@ beforeEach(() => { ;(getTestGroups as jest.Mock).mockReturnValue([]) }) +beforeEach(() => { + mockScreenSize("desktop") +}) + afterAll(() => { global.scrollTo = originalScrollTo }) diff --git a/assets/tests/components/mapPage/mapDisplay.test.tsx b/assets/tests/components/mapPage/mapDisplay.test.tsx index 8a28dbdad..f279a5011 100644 --- a/assets/tests/components/mapPage/mapDisplay.test.tsx +++ b/assets/tests/components/mapPage/mapDisplay.test.tsx @@ -34,7 +34,10 @@ import vehicleFactory, { } from "../../factories/vehicle" import { setHtmlWidthHeightForLeafletMap } from "../../testHelpers/leafletMapWidth" -import { mockUsePatternsByIdForVehicles } from "../../testHelpers/mockHelpers" +import { + mockScreenSize, + mockUsePatternsByIdForVehicles, +} from "../../testHelpers/mockHelpers" import shapeFactory from "../../factories/shape" import { zoomInButton } from "../../testHelpers/selectors/components/map" @@ -49,6 +52,8 @@ import { } from "../../testHelpers/selectors/components/mapPage/map" import { fullStoryEvent } from "../../../src/helpers/fullStory" import { recenterControl } from "../../testHelpers/selectors/components/map/controls/recenterControl" +import getTestGroups from "../../../src/userTestGroups" +import { TestGroups } from "../../../src/userInTestGroup" jest.mock("userTestGroups", () => ({ __esModule: true, @@ -98,6 +103,7 @@ jest.mock("../../../src/helpers/fullStory") beforeEach(() => { jest.spyOn(global, "scrollTo").mockImplementationOnce(jest.fn()) + mockScreenSize("desktop") }) type VehicleIdToVehicle = { @@ -463,6 +469,134 @@ describe("", () => { container.querySelector(".c-vehicle-map__route-shape") ).toBeInTheDocument() }) + + test("right-clicking brings up the detour dropdown menu", async () => { + jest + .mocked(getTestGroups) + .mockReturnValue([TestGroups.DetoursPilot]) + + const route = routeFactory.build() + const runId = runIdFactory.build() + + const selectedVehicle = randomLocationVehicle.build({ + routeId: route.id, + runId: runId, + }) + + setHtmlWidthHeightForLeafletMap() + mockUseVehicleForId([selectedVehicle]) + mockUseVehiclesForRouteMap({ [route.id]: [selectedVehicle] }) + mockUsePatternsByIdForVehicles([selectedVehicle], { + stopCount: 8, + }) + + render( + + ) + + await userEvent.pointer({ + keys: "[MouseRight>]", + target: screen.getByText(runId), + }) + + expect( + screen.getByRole("button", { + name: `Start a detour on route ${route.id}`, + }) + ).toBeVisible() + }) + + test("right-clicking does not bring up the detour dropdown if the user isn't a member of the test group", async () => { + jest.mocked(getTestGroups).mockReturnValue([]) + + const route = routeFactory.build() + const runId = runIdFactory.build() + + const selectedVehicle = randomLocationVehicle.build({ + routeId: route.id, + runId: runId, + }) + + setHtmlWidthHeightForLeafletMap() + mockUseVehicleForId([selectedVehicle]) + mockUseVehiclesForRouteMap({ [route.id]: [selectedVehicle] }) + mockUsePatternsByIdForVehicles([selectedVehicle], { + stopCount: 8, + }) + + render( + + ) + + await userEvent.pointer({ + keys: "[MouseRight>]", + target: screen.getByText(runId), + }) + + expect( + screen.queryByRole("button", { + name: `Start a detour on route ${route.id}`, + }) + ).not.toBeInTheDocument() + }) + + test("right-clicking does not bring up the detour dropdown on mobile", async () => { + jest + .mocked(getTestGroups) + .mockReturnValue([TestGroups.DetoursPilot]) + mockScreenSize("mobile") + + const route = routeFactory.build() + const runId = runIdFactory.build() + + const selectedVehicle = randomLocationVehicle.build({ + routeId: route.id, + runId: runId, + }) + + setHtmlWidthHeightForLeafletMap() + mockUseVehicleForId([selectedVehicle]) + mockUseVehiclesForRouteMap({ [route.id]: [selectedVehicle] }) + mockUsePatternsByIdForVehicles([selectedVehicle], { + stopCount: 8, + }) + + render( + + ) + + await userEvent.pointer({ + keys: "[MouseRight>]", + target: screen.getByText(runId), + }) + + expect( + screen.queryByRole("button", { + name: `Start a detour on route ${route.id}`, + }) + ).not.toBeInTheDocument() + }) }) test("and vehicle is a shuttle; should display: vehicle icon, but not route shape or stops", () => {