CODEX+JGARRETT: refactor POIController (#54)

* move POI overlay logic out of map

* split POI controller into two

* add docs

* remove comment

Author: johngarrett

src/main.ts

@@ -3,7 +3,8 @@ import { initMap } from "./map/map";
 import {
     POIBoundsController,
     POICollisionController,
-    POIController,
+    POIMarkerController,
+    POIPopupController,
     POIs,
     POITracker,
 } from "./points";
@@ -49,9 +50,14 @@ const locationController = new LocationController({
 });
 
 /**
- * controls showing the popup and marking the circles
+ * popup/audio controller for the currently active POI
  */
-new POIController({ poiTracker });
+new POIPopupController({ poiTracker });
+
+/**
+ * on-map marker layer for POIs
+ */
+const poiMarkerController = new POIMarkerController({ poiTracker, POIs });
 
 /**
  * controls collision between live user location and POI bounds
@@ -68,7 +74,6 @@ new POICollisionController({
 const boundsController = new POIBoundsController({ POIs });
 
 const map = initMap({
-    POIs,
     config: {
         initialLocation: [34.181922, -116.414579],
         initialZoom: 21,
@@ -86,7 +91,11 @@ const map = initMap({
         },
     },
     // TODO: baseclass for exposing a layer?
-    additionalLayers: [locationController.layer, boundsController.layer],
+    additionalLayers: [
+        locationController.layer,
+        boundsController.layer,
+        poiMarkerController.layer,
+    ],
     providers: {
         poiTracker,
         locationController,

src/map/map.ts

@@ -1,6 +1,4 @@
-import type { POI } from "../types";
 import L, { Layer, TileLayer, type MapOptions } from "leaflet";
-import { poiMarker } from "./components/poi-marker";
 import { POITracker } from "../points";
 import { debug } from "../utils";
 import type { LocationController } from "../location/location-controller";
@@ -26,7 +24,6 @@ type MapParameters = {
         locationController: LocationController;
     };
     additionalLayers?: Layer[];
-    POIs: Array<POI>;
 };
 
 export const initMap = (params: MapParameters) => {
@@ -53,19 +50,6 @@ export const initMap = (params: MapParameters) => {
         L.control.layers(config.tileLayers).addTo(map);
     }
 
-    // TODO: move this into a layer exposed on the POIControlller
-    params.POIs.forEach((POI, index) => {
-        const { latitude, longitude } = POI.location;
-
-        L.marker([latitude, longitude], {
-            icon: poiMarker({ number: index + 1, POI }),
-        })
-            .addTo(map)
-            .on("click", () => {
-                poiTracker.select(POI);
-            });
-    });
-
     // Safari (macOS/iOS) can change viewport when the location permission dialog
     // appears or closes, so Leaflet’s cached size becomes wrong. Recompute it.
     const recomputeMapSize = () => {

src/points/POI-marker-controller.ts

@@ -0,0 +1,40 @@
+import L from "leaflet";
+import type { POI } from "../types";
+import { getElementOrThrow } from "../utils";
+import type { POITracker } from "./POI-tracker";
+import { markerIdForPOI, poiMarker } from "./poi-marker";
+
+/**
+ * Owns the Leaflet marker layer for POIs.
+ *
+ * Responsibilities:
+ * - create the on-map POI markers
+ * - translate marker clicks into POITracker selections
+ * - apply persistent selected-marker styling when POITracker changes
+ */
+export class POIMarkerController {
+    constructor({ poiTracker, POIs }: { poiTracker: POITracker; POIs: POI[] }) {
+        this.layer = L.layerGroup(
+            POIs.map((POI, index) => {
+                const { latitude, longitude } = POI.location;
+
+                return L.marker([latitude, longitude], {
+                    icon: poiMarker({ number: index + 1, POI }),
+                }).on("click", () => {
+                    poiTracker.select(POI);
+                });
+            }),
+        );
+
+        poiTracker.addListener((activePOI) => {
+            if (activePOI) {
+                const nextMarker = getElementOrThrow({
+                    id: markerIdForPOI(activePOI),
+                });
+                nextMarker.style.opacity = "0.7";
+            }
+        });
+    }
+
+    public layer: L.LayerGroup;
+}

src/points/POI-controller.ts src/points/POI-popup-controller.tssrc/points/POI-controller.ts renamed to src/points/POI-popup-controller.ts

@@ -1,28 +1,27 @@
-import { markerIdForPOI } from "../map/components/poi-marker";
-import { POITracker } from "../points";
+import type { POITracker } from "./POI-tracker";
 import type { POI } from "../types";
 import { debug, getElementOrThrow, info } from "../utils";
 import { AudioController } from "./audio-controller";
 
 /**
- * Top-level coordinator for POI popup content.
+ * Owns the popup UI for the active POI.
  *
- * Audio stack breakdown:
- * - POIController shows and hides popup content for the active POI.
- * - AudioController owns the custom player UI inside the popup.
- * - AudioElement wraps the backing HTML audio node and handles media lifecycle.
+ * Responsibilities:
+ * - show and hide popup content for the active POI
+ * - populate popup title and image state
+ * - coordinate popup audio setup and teardown
  *
  * Provenance:
  * - the current custom POI audio player integration was AI-generated
  * - review behavior carefully when changing popup/audio interactions
  */
-export class POIController {
-    constructor(params: { poiTracker: POITracker }) {
+export class POIPopupController {
+    constructor({ poiTracker }: { poiTracker: POITracker }) {
         getElementOrThrow({ id: "poi-popup-close" }).addEventListener(
             "click",
             () => {
                 // when the user hits close, they're deselecting the active
-                params.poiTracker.deselectActive();
+                poiTracker.deselectActive();
             },
         );
 
@@ -47,9 +46,9 @@ export class POIController {
         };
         this.audioController = new AudioController();
 
-        params.poiTracker.addListener((activePOI) => {
+        poiTracker.addListener((activePOI) => {
             debug(
-                `[POIController] listener for poiTrackerInstance fired: ${activePOI}`,
+                `[POIPopupController] listener for poiTrackerInstance fired: ${activePOI}`,
             );
             if (activePOI) {
                 this.display(activePOI);
@@ -59,29 +58,21 @@ export class POIController {
         });
     }
 
-    /**
-     * called whenever POITracker sees a switch
-     */
+    /** Called when POITracker selects or switches to a POI. */
     display(poi: POI) {
         if (this.active?.poi && poi.id !== this.active.poi?.id) {
             info(
-                `[POIController] switching from ${this.active.poi?.id} to ${poi.id}`,
+                `[POIPopupController] switching from ${this.active.poi?.id} to ${poi.id}`,
             );
 
             this.audioController.teardown();
         }
 
         this.active = { poi: poi };
 
-        // --------- configure marker
-
-        const poiMarker = getElementOrThrow({ id: markerIdForPOI(poi) });
-        poiMarker.style.opacity = "0.7";
-
-        // --------- configure popup
-
         this.elements.title.textContent = poi.title;
 
+        // TODO: non optional
         if (poi.imageName) {
             this.elements.image.classList.add("poi-popup-image-loading");
             this.elements.image.removeAttribute("src");
@@ -106,7 +97,6 @@ export class POIController {
     }
 
     close() {
-        // TODO: hidden check here?
         this.elements.container.classList.add("hidden");
         this.hidden = true;
 

src/points/audio-controller.ts

@@ -22,7 +22,7 @@ const formatRemainingTime = (currentTime: number, duration: number) => {
  * Owns the custom POI audio player UI.
  *
  * Audio stack breakdown:
- * - POIController decides when popup audio should be shown for an active POI.
+ * - POIPopupController decides when popup audio should be shown for an active POI.
  * - AudioController owns the custom player UI inside the popup.
  * - AudioElement wraps the backing HTML audio node and handles media lifecycle.
  *

src/points/audio-element.ts

@@ -22,7 +22,7 @@ const getTimestampKey = (poi: POI) => {
  * Wraps the backing HTMLAudioElement used by the POI popup.
  *
  * Audio stack breakdown:
- * - POIController decides when popup audio should be shown for an active POI.
+ * - POIPopupController decides when popup audio should be shown for an active POI.
  * - AudioController owns the custom player UI inside the popup.
  * - AudioElement wraps the backing HTML audio node and handles media lifecycle.
  *

src/points/index.ts

@@ -1,5 +1,6 @@
 export * from "./POIs";
 export * from "./POI-bounds-controller";
 export * from "./POI-collision-controller";
-export * from "./POI-controller";
+export * from "./POI-marker-controller";
+export * from "./POI-popup-controller";
 export * from "./POI-tracker";

src/map/components/poi-marker.ts src/points/poi-marker.tssrc/map/components/poi-marker.ts renamed to src/points/poi-marker.ts

@@ -1,5 +1,5 @@
 import { type DivIcon, divIcon } from "leaflet";
-import type { POI } from "../../types";
+import type { POI } from "../types";
 
 type IconConfiguration = {
     POI: Pick<POI, "id">;