diff --git a/apps/admin/src/app/globals.css b/apps/admin/src/app/globals.css index d973710..48648ef 100644 --- a/apps/admin/src/app/globals.css +++ b/apps/admin/src/app/globals.css @@ -68,3 +68,8 @@ .leaflet-container { font-family: var(--font-body); } + +/* The toast's success icon sits on moss in admin.css; an error must not read as a success. */ +.toast-wrap.error .ico { + background: var(--danger); +} diff --git a/apps/admin/src/components/map/leaflet-map.test.tsx b/apps/admin/src/components/map/leaflet-map.test.tsx index 208446a..b6e9de2 100644 --- a/apps/admin/src/components/map/leaflet-map.test.tsx +++ b/apps/admin/src/components/map/leaflet-map.test.tsx @@ -1,4 +1,5 @@ import { fireEvent, render } from "@testing-library/react" +import userEvent from "@testing-library/user-event" import L from "leaflet" import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" @@ -147,6 +148,84 @@ describe("LeafletMap", () => { expect(onPinTap).toHaveBeenCalledWith(PINS[2]) }) + it("updates a pin's tooltip when its title changes under the same id", () => { + const { container, rerender } = render() + hoverAndReadTooltip(container, markerIcons(container)[0]!) + + rerender() + const tip = hoverAndReadTooltip(container, markerIcons(container)[0]!) + expect(tip?.textContent).toBe("Filled pothole · Los Angeles") + }) + + it("binds a tooltip when a pin gains a title and drops it when the title goes", () => { + const bare: MapPin = { id: "x", lat: 34, lng: -118 } + const { container, rerender } = render() + + rerender() + expect(hoverAndReadTooltip(container, markerIcons(container)[0]!)?.textContent).toBe("Now titled") + + fireEvent.mouseOut(markerIcons(container)[0]!) + rerender() + expect(hoverAndReadTooltip(container, markerIcons(container)[0]!)).toBeNull() + }) + + it("hands onPinTap the pin's latest data after a refetch", () => { + const onPinTap = vi.fn() + const { container, rerender } = render() + const refetched: MapPin = { ...PINS[2]!, tip: "Park cleanup (moved)" } + + rerender() + fireEvent.click(markerIcons(container)[2]!) + expect(onPinTap).toHaveBeenCalledWith(refetched) + }) + + it("names each interactive marker by its tooltip text", () => { + const { container } = render() + + const icons = markerIcons(container) + expect(icons.map((el) => el.getAttribute("role"))).toEqual(["button", "button", "button"]) + expect(icons.map((el) => el.getAttribute("aria-label"))).toEqual([ + "Deep pothole · Los Angeles", + "Tagged wall", + "Park cleanup", + ]) + }) + + it("renames a marker when its title changes", () => { + const { container, rerender } = render() + + rerender() + expect(markerIcons(container)[0]).toHaveAttribute("aria-label", "Filled pothole · Los Angeles") + }) + + it("keeps markers of a non-interactive map out of the tab order", () => { + const { container } = render() + + for (const el of markerIcons(container)) { + expect(el).not.toHaveAttribute("tabindex") + expect(el).not.toHaveAttribute("role") + } + }) + + it.each(["{Enter}", " "])("activates a focused marker with %j like a tap", async (key) => { + const onPinTap = vi.fn() + const { container } = render() + + const icon = markerIcons(container)[1]! + icon.focus() + await userEvent.setup().keyboard(key) + expect(onPinTap).toHaveBeenCalledTimes(1) + expect(onPinTap).toHaveBeenCalledWith(PINS[1]) + }) + + it("moves the view when the center or zoom props change", () => { + const setView = vi.spyOn(L.Map.prototype, "setView") + const { rerender } = render() + + rerender() + expect(setView).toHaveBeenLastCalledWith([40.7, -74], 12) + }) + it("removes the Leaflet map on unmount", () => { const remove = vi.spyOn(L.Map.prototype, "remove") const { unmount } = render() diff --git a/apps/admin/src/components/map/leaflet-map.tsx b/apps/admin/src/components/map/leaflet-map.tsx index 33d5176..f3e9895 100644 --- a/apps/admin/src/components/map/leaflet-map.tsx +++ b/apps/admin/src/components/map/leaflet-map.tsx @@ -3,6 +3,7 @@ import * as React from "react" import L from "leaflet" +import { isKeyboardActivationKey } from "@/components/shared/keyboard-activation" import { withCartoKey } from "@/lib/carto" import { CATEGORY_GLYPHS } from "@/lib/category" @@ -119,6 +120,22 @@ function tooltipNode(text: string): HTMLElement { return el } +const TOOLTIP_OPTIONS: L.TooltipOptions = { direction: "top", offset: [0, -30], className: "pi-map-tip" } + +// A keyboard marker is a role=button tab stop, and its divIcon has no text of its own to name it. +function labelMarker(marker: L.Marker, text: string | null): void { + const el = marker.getElement() + if (!el) return + if (text) el.setAttribute("aria-label", text) + else el.removeAttribute("aria-label") +} + +function syncTooltip(marker: L.Marker, text: string | null): void { + if (!text) marker.unbindTooltip() + else if (marker.getTooltip()) marker.setTooltipContent(tooltipNode(text)) + else marker.bindTooltip(tooltipNode(text), TOOLTIP_OPTIONS) +} + export interface LeafletMapProps { pins?: MapPin[] center?: [number, number] @@ -146,11 +163,16 @@ export function LeafletMap({ const mapRef = React.useRef(null) const tileRef = React.useRef(null) const markersRef = React.useRef>({}) - // Per-id memo of the last-rendered visual descriptor (category|draft|kind|active) and position, so - // reconcile can skip the expensive DivIcon rebuild + DOM teardown (setIcon) and the setLatLng call - // when nothing visible actually changed for that marker. Without this, a single activeId change - // re-icons and DOM-replaces ALL N markers; with it, only the de-activated + newly-active markers do. - const renderRef = React.useRef>({}) + // Per-id memo of the last-rendered visual descriptor (category|draft|kind|active), tooltip text and + // position, so reconcile can skip the expensive DivIcon rebuild + DOM teardown (setIcon), the tooltip + // rebind and the setLatLng call when nothing visible actually changed for that marker. Without this, a + // single activeId change re-icons and DOM-replaces ALL N markers; with it, only the de-activated + + // newly-active markers do. + const renderRef = React.useRef< + Record + >({}) + // The latest pin per id: marker handlers are bound once, and a refetch replaces the pin objects. + const pinsRef = React.useRef>({}) // Keep the latest onPinTap without re-running the create effect. const onPinTapRef = React.useRef(onPinTap) onPinTapRef.current = onPinTap @@ -197,11 +219,17 @@ export function LeafletMap({ mapRef.current = null markersRef.current = {} renderRef.current = {} + pinsRef.current = {} } // Intentionally run once on mount. // eslint-disable-next-line react-hooks/exhaustive-deps }, []) + const [centerLat, centerLng] = center + React.useEffect(() => { + mapRef.current?.setView([centerLat, centerLng], zoom) + }, [centerLat, centerLng, zoom]) + // Swap tiles when tint changes. React.useEffect(() => { const map = mapRef.current @@ -230,43 +258,52 @@ export function LeafletMap({ markersRef.current[id]?.remove() delete markersRef.current[id] delete renderRef.current[id] + delete pinsRef.current[id] } }) pins.forEach((p) => { + pinsRef.current[p.id] = p const existing = markersRef.current[p.id] const active = String(p.id) === String(activeId) // One cheap string capturing everything pinIcon() depends on. Equal key => identical DivIcon, so // we can skip rebuilding the HTML/SVG and the setIcon DOM teardown entirely. const key = `${p.category}|${p.draft}|${p.kind}|${active}` + const text = tooltipText(p) if (existing) { const prev = renderRef.current[p.id] // Re-icon only when the visual descriptor changed (e.g. this pin just gained/lost active). if (!prev || prev.key !== key) { existing.setIcon(pinIcon(p.category, { active, draft: p.draft, kind: p.kind })) } + if (!prev || prev.text !== text) { + syncTooltip(existing, text) + if (interactive) labelMarker(existing, text) + } // Re-position only when the coordinates actually moved. if (!prev || prev.lat !== p.lat || prev.lng !== p.lng) { existing.setLatLng([p.lat, p.lng]) } - renderRef.current[p.id] = { key, lat: p.lat, lng: p.lng } } else { const icon = pinIcon(p.category, { active, draft: p.draft, kind: p.kind }) - const m = L.marker([p.lat, p.lng], { icon, riseOnHover: true }).addTo(map) - const tip = tooltipText(p) - if (tip) { - m.bindTooltip(tooltipNode(tip), { - direction: "top", - offset: [0, -30], - className: "pi-map-tip", - }) + const m = L.marker([p.lat, p.lng], { icon, riseOnHover: true, keyboard: interactive }).addTo(map) + syncTooltip(m, text) + if (interactive) labelMarker(m, text) + const tap = () => { + const latest = pinsRef.current[p.id] + if (latest) onPinTapRef.current?.(latest) } - m.on("click", () => onPinTapRef.current?.(p)) + m.on("click", tap) + m.on("keydown", (e: L.LeafletKeyboardEvent) => { + if (!isKeyboardActivationKey(e.originalEvent.key)) return + e.originalEvent.preventDefault() + tap() + }) markersRef.current[p.id] = m - renderRef.current[p.id] = { key, lat: p.lat, lng: p.lng } } + renderRef.current[p.id] = { key, text, lat: p.lat, lng: p.lng } }) - }, [pins, activeId]) + }, [pins, activeId, interactive]) return
} diff --git a/apps/admin/src/components/map/live-map.test.tsx b/apps/admin/src/components/map/live-map.test.tsx new file mode 100644 index 0000000..73aa9a5 --- /dev/null +++ b/apps/admin/src/components/map/live-map.test.tsx @@ -0,0 +1,126 @@ +import { act, screen, waitFor, within } from "@testing-library/react" +import userEvent from "@testing-library/user-event" +import type { HomeMapPin } from "@civfix/shared" +import { describe, expect, it, vi } from "vitest" + +import type * as ApiModule from "@/lib/api" +import type { LeafletMapProps } from "@/components/map/leaflet-map" +import { LiveMap } from "@/components/map/live-map" +import { queryKeys } from "@/lib/query" +import { apiMock } from "@/test/api-mock" +import { renderWithQuery } from "@/test/render" + +vi.mock("@/lib/api", async (importOriginal) => { + const { apiMock } = await import("@/test/api-mock") + return { ...(await importOriginal()), api: apiMock } +}) + +vi.mock("@/components/map/leaflet-map", () => ({ + LeafletMap: ({ pins = [], onPinTap }: LeafletMapProps) => ( +
+ {pins.map((p) => ( + + ))} +
+ ), +})) + +function reportPin(over: Partial = {}): HomeMapPin { + return { + refType: "report", + id: "r1", + lat: 34, + lng: -118, + category: "graffiti", + status: "published", + flagged: false, + title: "Tagged wall", + place: "Echo Park", + ...over, + } +} + +function eventPin(over: Partial = {}): HomeMapPin { + return { + refType: "event", + id: "e1", + lat: 34.1, + lng: -118.1, + category: null, + eventKind: "cleanup", + status: "upcoming", + flagged: false, + title: "Park cleanup", + place: "Elysian Park", + ...over, + } +} + +async function renderMap(pins: HomeMapPin[]) { + apiMock.adminHomeMap.mockResolvedValue({ pins }) + const user = userEvent.setup() + const view = renderWithQuery() + return { user, ...view } +} + +async function tap(user: ReturnType, id: string): Promise { + await user.click(await screen.findByRole("button", { name: `pin ${id}` })) +} + +describe("LiveMap active card", () => { + it("titles an untitled report by its category", async () => { + const { user } = await renderMap([reportPin({ title: "" })]) + await tap(user, "report-r1") + + expect(screen.getByText("Graffiti report")).toBeInTheDocument() + }) + + it("titles an untitled report without a category as a report", async () => { + const { user } = await renderMap([reportPin({ title: "", category: null })]) + await tap(user, "report-r1") + + expect(screen.getByText("Report")).toBeInTheDocument() + }) + + it.each([ + ["cleanup", "Cleanup event"], + ["other_volunteer", "Other Volunteer event"], + ] as const)("labels a %s event pin by its kind", async (eventKind, label) => { + const { user } = await renderMap([eventPin({ eventKind })]) + await tap(user, "event-e1") + + expect(screen.getByText(label)).toBeInTheDocument() + }) + + it("follows the tapped pin's data when the feed refetches", async () => { + const { user, client } = await renderMap([reportPin()]) + await tap(user, "report-r1") + expect(screen.getByText("Tagged wall")).toBeInTheDocument() + + act(() => client.setQueryData(queryKeys.home.map, { pins: [reportPin({ title: "Wall repainted" })] })) + expect(await screen.findByText("Wall repainted")).toBeInTheDocument() + expect(screen.queryByText("Tagged wall")).toBeNull() + }) + + it("drops the card when the tapped pin leaves the feed", async () => { + const { user, client } = await renderMap([reportPin(), eventPin()]) + await tap(user, "report-r1") + + act(() => client.setQueryData(queryKeys.home.map, { pins: [eventPin()] })) + await waitFor(() => expect(screen.queryByText("Tagged wall")).toBeNull()) + expect(screen.queryByRole("button", { name: /Open report/ })).toBeNull() + }) + + it("announces the card politely and closes it from its Close button", async () => { + const { user } = await renderMap([reportPin()]) + await tap(user, "report-r1") + + const card = screen.getByText("Tagged wall").closest(".map-active-card") as HTMLElement + expect(card.closest('[aria-live="polite"]')).not.toBeNull() + + await user.click(within(card).getByRole("button", { name: "Close" })) + expect(screen.queryByText("Tagged wall")).toBeNull() + }) +}) diff --git a/apps/admin/src/components/map/live-map.tsx b/apps/admin/src/components/map/live-map.tsx index c74577d..990809a 100644 --- a/apps/admin/src/components/map/live-map.tsx +++ b/apps/admin/src/components/map/live-map.tsx @@ -7,7 +7,8 @@ import type { HomeMapPin } from "@civfix/shared" import { Icons } from "@/components/icons" import { useHomeMap } from "@/hooks/use-admin-home" import { useNav } from "@/store/ui-store" -import { EVENT_KIND_PIN_KIND } from "@/lib/event-kind" +import { categoryLabel } from "@/lib/category" +import { EVENT_KIND_PIN_KIND, eventKindLabel } from "@/lib/event-kind" import { BUCKET_VIEW, reportBucketOf, @@ -16,35 +17,44 @@ import { } from "@/lib/report-status" import type { MapPin, MapTint } from "@/components/map/leaflet-map" - const LeafletMap = dynamic(() => import("@/components/map/leaflet-map").then((m) => m.LeafletMap), { ssr: false, loading: () =>
, }) -function toMapPin(p: HomeMapPin): MapPin & { +// The feed sends an empty title for an untitled report. +function pinTitle(p: HomeMapPin): string { + if (p.title) return p.title + return p.category ? `${categoryLabel(p.category)} report` : "Report" +} + +export function toMapPin(p: HomeMapPin): MapPin & { refType: HomeMapPin["refType"] refId: string + eventKind: NonNullable | null status: HomeMapPin["status"] flagged: boolean title: string } { const isEvent = p.refType === "event" const needs = !isEvent && reportNeedsAttention(p.status, p.flagged) + const eventKind = isEvent ? (p.eventKind ?? "cleanup") : null + const title = pinTitle(p) return { id: `${p.refType}-${p.id}`, refType: p.refType, refId: p.id, + eventKind, lat: p.lat, lng: p.lng, category: isEvent ? "event" : p.category, draft: needs, - kind: isEvent ? EVENT_KIND_PIN_KIND[p.eventKind ?? "cleanup"] : null, - tip: p.title, + kind: eventKind ? EVENT_KIND_PIN_KIND[eventKind] : null, + tip: title, place: p.place, status: p.status, flagged: p.flagged, - title: p.title, + title, } } @@ -57,8 +67,11 @@ const BUCKET_TONE: Record = { removed: "var(--bloom-700)", } -function statusTone(m: ActivePin): { color: string; label: string } { - if (m.refType === "event") return { color: "var(--sun-700)", label: "Cleanup event" } +export function statusTone(m: ActivePin): { color: string; label: string } { + if (m.eventKind === "other_volunteer") { + return { color: "var(--moss-700)", label: `${eventKindLabel(m.eventKind)} event` } + } + if (m.eventKind) return { color: "var(--sun-700)", label: `${eventKindLabel(m.eventKind)} event` } if (m.flagged) return { color: "var(--bloom-700)", label: "Flagged" } const bucket = reportBucketOf(m.status) return { color: BUCKET_TONE[bucket], label: BUCKET_VIEW[bucket].label } @@ -67,10 +80,12 @@ function statusTone(m: ActivePin): { color: string; label: string } { export function LiveMap({ tint = "voyager" }: { tint?: MapTint }) { const nav = useNav() const q = useHomeMap() - const [active, setActive] = React.useState(null) + const [activeId, setActiveId] = React.useState(null) const pins = React.useMemo(() => q.data?.pins ?? [], [q.data]) const markers = React.useMemo(() => pins.map(toMapPin), [pins]) + const active = markers.find((m) => m.id === activeId) ?? null + const tone = active ? statusTone(active) : null const reportCount = pins.filter((p) => p.refType === "report").length const eventCount = pins.filter((p) => p.refType === "event").length const needsAttention = pins.filter( @@ -100,7 +115,7 @@ export function LiveMap({ tint = "voyager" }: { tint?: MapTint }) { pins={markers} tint={tint} activeId={active ? active.id : null} - onPinTap={(p) => setActive(p as ActivePin)} + onPinTap={(p) => setActiveId(p.id)} />
@@ -134,32 +149,38 @@ export function LiveMap({ tint = "voyager" }: { tint?: MapTint }) {
)} - {active && - (() => { - const tone = statusTone(active) - return ( -
-
- -
-
-
{active.title}
-
- {active.place} - · - {tone.label} -
+
+ {active && tone && ( +
+
+ +
+
+
{active.title}
+
+ {active.place} + · + {tone.label}
-
- ) - })()} + + +
+ )} +
) diff --git a/apps/admin/src/components/providers.test.tsx b/apps/admin/src/components/providers.test.tsx index 20a4010..e6b5d92 100644 --- a/apps/admin/src/components/providers.test.tsx +++ b/apps/admin/src/components/providers.test.tsx @@ -4,8 +4,16 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest" import { Providers } from "@/components/providers" vi.mock("@/components/auth/auth-hydrator", () => ({ AuthHydrator: () => null })) +const session = vi.hoisted(() => ({ + current: { isOperator: true, operator: null, status: "authenticated" } as { + isOperator: boolean + operator: null + status: string + }, +})) + vi.mock("@/hooks/use-admin-auth", () => ({ - useOperatorSession: () => ({ isOperator: true, operator: null, status: "authenticated" }), + useOperatorSession: () => session.current, })) function Crash(): never { @@ -18,6 +26,7 @@ beforeEach(() => { afterEach(() => { vi.restoreAllMocks() + session.current = { isOperator: true, operator: null, status: "authenticated" } }) describe("Providers", () => { @@ -45,4 +54,17 @@ describe("Providers", () => { expect(alert).not.toHaveTextContent("shell exploded") expect(within(alert).getByRole("button", { name: "Reload" })).toBeInTheDocument() }) + + it("shows a signing-out screen rather than the sign-in gate while signing out", () => { + session.current = { isOperator: false, operator: null, status: "signing-out" } + render( + +

Dashboard content

+
, + ) + + expect(screen.getByRole("status")).toHaveTextContent("Signing out...") + expect(screen.queryByText("Dashboard content")).toBeNull() + expect(screen.queryByRole("button", { name: "Try again" })).toBeNull() + }) }) diff --git a/apps/admin/src/components/providers.tsx b/apps/admin/src/components/providers.tsx index ade6b43..ef1b2a5 100644 --- a/apps/admin/src/components/providers.tsx +++ b/apps/admin/src/components/providers.tsx @@ -35,6 +35,7 @@ export function Providers({ children }: { children: React.ReactNode }) { /** * The operator gate. Only an authenticated operator session renders the dashboard: * - idle / loading -> a minimal loading screen (Access exchange in flight, or pre-hydration). + * - signing-out -> the same screen, saying so, until the Access logout navigation lands. * - not an operator -> the full-page Cloudflare Access gate (anonymous: authenticating + manual * continue; forbidden: not-authorized message). * - operator -> the dashboard shell (children). @@ -43,7 +44,11 @@ function AuthGate({ children }: { children: React.ReactNode }) { const { isOperator, status } = useOperatorSession() if (status === "idle" || status === "loading") { - return + return + } + + if (status === "signing-out") { + return } if (!isOperator) { @@ -53,8 +58,8 @@ function AuthGate({ children }: { children: React.ReactNode }) { return <>{children} } -/** Minimal centered loading screen shown while the session check is in flight. */ -function BootScreen() { +/** Minimal centered screen shown while the session is being established or ended. */ +function BootScreen({ label }: { label: string }) { return (
) } diff --git a/apps/admin/src/components/shared/data-states.test.tsx b/apps/admin/src/components/shared/data-states.test.tsx new file mode 100644 index 0000000..f7b6a55 --- /dev/null +++ b/apps/admin/src/components/shared/data-states.test.tsx @@ -0,0 +1,29 @@ +import { render, screen } from "@testing-library/react" +import { AppError, ErrorCode } from "@civfix/shared" +import { describe, expect, it } from "vitest" + +import { ErrorState } from "@/components/shared/data-states" + +describe("ErrorState", () => { + it("shows the API's own message", () => { + render() + expect(screen.getByRole("alert")).toHaveTextContent("Operators only") + }) + + it("explains a failed request in plain words instead of the browser's fetch text", () => { + render() + const alert = screen.getByRole("alert") + expect(alert).toHaveTextContent("Could not reach the server. Check your connection and try again.") + expect(alert).not.toHaveTextContent("Failed to fetch") + }) + + it("shows the generic copy for a thrown non-error value", () => { + render() + expect(screen.getByRole("alert")).toHaveTextContent("Something went wrong. Please try again.") + }) + + it("lets an explicit message replace the error's", () => { + render() + expect(screen.getByRole("alert")).toHaveTextContent("Custom copy") + }) +}) diff --git a/apps/admin/src/components/shared/data-states.tsx b/apps/admin/src/components/shared/data-states.tsx index 1780627..2434909 100644 --- a/apps/admin/src/components/shared/data-states.tsx +++ b/apps/admin/src/components/shared/data-states.tsx @@ -2,7 +2,7 @@ import * as React from "react" -import { toAppError } from "@/lib/api" +import { errorMessage } from "@/lib/error-messages" /** * Standard loading / error / empty state components for data-bound views. The prototype had NONE of @@ -30,8 +30,7 @@ export function LoadingState({ label = "Loading..." }: { label?: string }) { /** * Error panel with an optional retry and an optional primary action beside it (the error boundary's - * Reload). Surfaces the normalized AppError message unless `message` replaces it; when the backend is - * down this renders the friendly INTERNAL message rather than crashing. + * Reload). Shows the error's operator copy from errorMessage unless `message` replaces it. */ export function ErrorState({ error, @@ -46,7 +45,7 @@ export function ErrorState({ message?: string action?: { label: string; onClick: () => void } }) { - const errorText = React.useMemo(() => toAppError(error).message, [error]) + const errorText = React.useMemo(() => errorMessage(error), [error]) const retry = onRetry && (
- {current.body &&

{current.body}

} + {current.body && ( +

+ {current.body} +

+ )} {current.kind === "prompt" && ( <> - {current.label && } + {current.label && ( + + )}