Admin correctness and accessibility fixes

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);
+}

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(<LeafletMap pins={PINS} />)
+    hoverAndReadTooltip(container, markerIcons(container)[0]!)
+
+    rerender(<LeafletMap pins={[{ ...PINS[0]!, tip: "Filled pothole" }, ...PINS.slice(1)]} />)
+    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(<LeafletMap pins={[bare]} />)
+
+    rerender(<LeafletMap pins={[{ ...bare, tip: "Now titled" }]} />)
+    expect(hoverAndReadTooltip(container, markerIcons(container)[0]!)?.textContent).toBe("Now titled")
+
+    fireEvent.mouseOut(markerIcons(container)[0]!)
+    rerender(<LeafletMap pins={[bare]} />)
+    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(<LeafletMap pins={PINS} onPinTap={onPinTap} />)
+    const refetched: MapPin = { ...PINS[2]!, tip: "Park cleanup (moved)" }
+
+    rerender(<LeafletMap pins={[PINS[0]!, PINS[1]!, refetched]} onPinTap={onPinTap} />)
+    fireEvent.click(markerIcons(container)[2]!)
+    expect(onPinTap).toHaveBeenCalledWith(refetched)
+  })
+
+  it("names each interactive marker by its tooltip text", () => {
+    const { container } = render(<LeafletMap pins={PINS} />)
+
+    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(<LeafletMap pins={PINS} />)
+
+    rerender(<LeafletMap pins={[{ ...PINS[0]!, tip: "Filled pothole" }]} />)
+    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(<LeafletMap pins={PINS} interactive={false} />)
+
+    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(<LeafletMap pins={PINS} onPinTap={onPinTap} />)
+
+    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(<LeafletMap pins={PINS} center={[34, -118]} zoom={10} />)
+
+    rerender(<LeafletMap pins={PINS} center={[40.7, -74]} zoom={12} />)
+    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(<LeafletMap pins={PINS} />)

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<L.Map | null>(null)
   const tileRef = React.useRef<L.TileLayer | null>(null)
   const markersRef = React.useRef<Record<string, L.Marker>>({})
-  // 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<Record<string, { key: string; lat: number; lng: number }>>({})
+  // 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<string, { key: string; text: string | null; lat: number; lng: number }>
+  >({})
+  // The latest pin per id: marker handlers are bound once, and a refetch replaces the pin objects.
+  const pinsRef = React.useRef<Record<string, MapPin>>({})
   // 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 <div ref={elRef} className="pi-map-canvas" />
 }

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<typeof ApiModule>()), api: apiMock }
+})
+
+vi.mock("@/components/map/leaflet-map", () => ({
+  LeafletMap: ({ pins = [], onPinTap }: LeafletMapProps) => (
+    <div>
+      {pins.map((p) => (
+        <button key={p.id} type="button" onClick={() => onPinTap?.(p)}>
+          {`pin ${p.id}`}
+        </button>
+      ))}
+    </div>
+  ),
+}))
+
+function reportPin(over: Partial<HomeMapPin> = {}): 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> = {}): 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(<LiveMap />)
+  return { user, ...view }
+}
+
+async function tap(user: ReturnType<typeof userEvent.setup>, id: string): Promise<void> {
+  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()
+  })
+})