From 0b8cb4f33f1ac0fc541f5d9d5c348ea93342d83e Mon Sep 17 00:00:00 2001
From: Sameh Khamis <sameh@halfpixel.ai>
Date: Sat, 31 Oct 2026 19:28:02 -0700
Subject: [PATCH] refactor(ui): remove the map stack chip DOM builders

The dock renders the chips from the map source controls' published view,
so renderMapStackChips, syncMapStackChips and MAP_STACK_CHIP_CLASS have no
callers. Their tests now pin the chip models; the dock test keeps the
rendered chips' button type and native enablement.
---
 CHANGELOG.md                |   4 +-
 docs/CODE-BOUNDARIES.md     |   6 +-
 src/mapStackChips.js        | 116 ++--------------
 src/mapStackChips.test.mjs  | 263 +++++++-----------------------------
 src/ui/commandDock.test.mjs |   5 +
 src/ui/mapSource.js         |   3 -
 6 files changed, 71 insertions(+), 326 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 954779142..5b555f592 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -65,8 +65,8 @@
 - `createMapSourceControls()` (`./ui/maps`) no longer renders the map source
   chips into a `container` or writes a `statusElement`: it presents the chip
   row and status badge and hands them to its `publish` option, and Core's
-  dock renders them and calls `select()`. `renderMapStackChips` and
-  `syncMapStackChips` are deprecated (samehkhamis).
+  dock renders them and calls `select()`. `renderMapStackChips`,
+  `syncMapStackChips` and `MAP_STACK_CHIP_CLASS` are removed (samehkhamis).
 
 - `LocationControls` (`./ui/location`) no longer builds the city and
   landmark pills or writes the mini status: it presents the tray, including
diff --git a/docs/CODE-BOUNDARIES.md b/docs/CODE-BOUNDARIES.md
index ed77a4ecf..f0bb9e979 100644
--- a/docs/CODE-BOUNDARIES.md
+++ b/docs/CODE-BOUNDARIES.md
@@ -399,9 +399,9 @@ remain in their callers.
 `ui/maps` owns source-chip presentation, selection feedback and its subscription
 lifetime. It receives the existing controller and explicit state/action callbacks;
 it imports no renderer or application. Source construction and availability policy
-remain with the map controller. Rebuilding controls removes their previous chip
-listeners, and destruction suppresses late completions without owning or destroying
-the supplied controller.
+remain with the map controller. The dock renders the chips; rebuilding controls
+drops their previous subscription, and destruction suppresses late completions
+without owning or destroying the supplied controller.
 
 ## Layer panel
 
diff --git a/src/mapStackChips.js b/src/mapStackChips.js
index 5798e9fcf..fed2880b3 100644
--- a/src/mapStackChips.js
+++ b/src/mapStackChips.js
@@ -1,19 +1,18 @@
 // MAP STACK source chips — the always-visible replacement for the `<select>`
-// that used to sit in the Map Stack panel. One button per stack, rendered from
-// `MapStackController.getStacks()`. The approved sources below are
-// the whole shipped set; keeping the allowlist explicit means a stack added to
-// `MAP_STACKS` for another purpose cannot reach the tray until someone names it
-// here.
+// that used to sit in the Map Stack panel. One chip model per stack, from
+// `MapStackController.getStacks()`; the dock renders them. The approved sources
+// below are the whole shipped set; keeping the allowlist explicit means a stack
+// added to `MAP_STACKS` for another purpose cannot reach the tray until someone
+// names it here.
 //
-// The chips are a control SURFACE only: selecting one calls back into the same
-// `_setMapStack()` path the dropdown's `change` handler used, and the active
-// state is re-synced from controller state (never optimistically), so a failed
-// or superseded switch still leaves the truly-active stack lit.
+// The chips are a control SURFACE only: selecting one calls
+// `createMapSourceControls().select()`, the path `_setMapStack()` uses, and the
+// active state is re-synced from controller state (never optimistically), so a
+// failed or superseded switch still leaves the truly-active stack lit.
 
 import { keySetupRequirementEnvVars } from './keySetupCore.mjs';
 import { t } from './i18n/index.js';
 
-export const MAP_STACK_CHIP_CLASS = 'map-stack-chip';
 export const PRESENTED_MAP_STACK_IDS = Object.freeze([
   'photoreal',
   'bing-aerial',
@@ -78,100 +77,3 @@ export function mapStackChipModels(stacks, activeId) {
     .filter(Boolean)
     .map((stack) => mapStackChipModel(stack, activeId));
 }
-
-/**
- * Renders the chip row into `container`, replacing any previous chips.
- * @deprecated Core's dock renders the row from `createMapSourceControls()`'s
- *   published view; this DOM builder is no longer used by Core.
- * @param {HTMLElement} container - Row element.
- * @param {Array<object>} stacks - `MapStackController.getStacks()` output.
- * @param {object} [options]
- * @param {string|null} [options.activeId] - Currently active stack id.
- * @param {(stackId: string) => void} [options.onSelect] - Selection callback.
- * @param {Document} [options.doc] - Document override (tests).
- * @param {(element: HTMLElement, type: string, listener: Function) => void} [options.bind] - Listener owner override.
- * @returns {Array<object>} The rendered chip models.
- */
-export function renderMapStackChips(
-  container,
-  stacks,
-  {
-    activeId = null,
-    onSelect = null,
-    doc,
-    bind = (element, type, listener) =>
-      element.addEventListener(type, listener),
-  } = {},
-) {
-  if (!container) return [];
-  const ownerDoc = doc || container.ownerDocument || globalThis.document;
-  if (!ownerDoc?.createElement) return [];
-
-  container.innerHTML = '';
-  const models = mapStackChipModels(stacks, activeId);
-
-  for (const model of models) {
-    const chip = ownerDoc.createElement('button');
-    chip.type = 'button';
-    chip.className = [
-      MAP_STACK_CHIP_CLASS,
-      model.active ? 'active' : '',
-      model.available ? '' : 'unavailable',
-    ]
-      .filter(Boolean)
-      .join(' ');
-    chip.dataset.stackId = model.id;
-    chip.title = model.title;
-    chip.setAttribute('aria-pressed', String(model.active));
-    chip.setAttribute('aria-disabled', String(!model.available));
-    if (!model.available) {
-      chip.setAttribute(
-        'aria-label',
-        t('{label} unavailable: {hint}', {
-          label: model.label,
-          hint: model.unavailableHint,
-        }),
-      );
-    }
-
-    const label = ownerDoc.createElement('span');
-    label.className = 'map-stack-chip-label';
-    label.textContent = model.label;
-    chip.appendChild(label);
-
-    if (model.requirement) {
-      const requirement = ownerDoc.createElement('span');
-      requirement.className = 'map-stack-chip-req';
-      requirement.textContent = model.requirement;
-      chip.appendChild(requirement);
-    }
-
-    bind(chip, 'click', () => {
-      if (!model.available) return;
-      onSelect?.(model.id);
-    });
-    container.appendChild(chip);
-  }
-
-  return models;
-}
-
-/**
- * Re-points the active chip at controller state.
- * @deprecated Core's dock renders the active chip from the published view. Availability never changes at
- * runtime (it tracks the ion token), so only the active/pressed pair is synced.
- * @param {HTMLElement} container - Row element.
- * @param {string|null} activeId - Currently active stack id.
- * @returns {void}
- */
-export function syncMapStackChips(container, activeId) {
-  const chips = container?.children;
-  if (!chips) return;
-  for (const chip of Array.from(chips)) {
-    const stackId = chip?.dataset?.stackId;
-    if (!stackId) continue;
-    const active = stackId === activeId;
-    chip.classList?.toggle('active', active);
-    chip.setAttribute?.('aria-pressed', String(active));
-  }
-}
diff --git a/src/mapStackChips.test.mjs b/src/mapStackChips.test.mjs
index 3a2c89e66..0d3ba21dc 100644
--- a/src/mapStackChips.test.mjs
+++ b/src/mapStackChips.test.mjs
@@ -4,83 +4,19 @@ import { readStylesheet } from './testSupport/readStylesheet.mjs';
 // MAP STACK chip row — the dropdown's replacement control surface.
 //
 // The owner's complaint was two clicks (open panel → open dropdown) to change
-// basemap. These tests pin the three things that make the row a faithful swap:
-// it projects the owner-approved four-source allowlist from the controller's
-// stack data, a click dispatches the same selection the `change` handler used
-// to, and the lit chip tracks controller state rather than the click. Run with:
-// npm test
+// basemap. These tests pin the chip models: the owner-approved allowlist
+// projected from the controller's stack data, one active chip, and the reason
+// an unavailable chip gives. The dock's rendering and clicks are pinned in
+// src/ui/commandDock.test.mjs. Run with: npm test
 import { test } from 'node:test';
 import assert from 'node:assert/strict';
 import { readFileSync } from 'node:fs';
 import {
-  MAP_STACK_CHIP_CLASS,
   PRESENTED_MAP_STACK_IDS,
   mapStackChipModel,
   mapStackChipModels,
-  renderMapStackChips,
-  syncMapStackChips,
 } from './mapStackChips.js';
-
-/** Minimal element stand-in — the row only needs create/append/attr/class. */
-function makeElement(tagName = 'div') {
-  const element = {
-    tagName,
-    type: '',
-    className: '',
-    title: '',
-    disabled: false,
-    textContent: '',
-    dataset: {},
-    attributes: {},
-    listeners: {},
-    children: [],
-    classList: {
-      toggle(name, force) {
-        const classes = new Set(
-          String(element.className).split(/\s+/).filter(Boolean),
-        );
-        const next = force === undefined ? !classes.has(name) : !!force;
-        if (next) classes.add(name);
-        else classes.delete(name);
-        element.className = [...classes].join(' ');
-      },
-      contains(name) {
-        return String(element.className).split(/\s+/).includes(name);
-      },
-    },
-    appendChild(child) {
-      element.children.push(child);
-      return child;
-    },
-    setAttribute(name, value) {
-      element.attributes[name] = String(value);
-    },
-    getAttribute(name) {
-      return element.attributes[name] ?? null;
-    },
-    addEventListener(type, handler) {
-      (element.listeners[type] ||= []).push(handler);
-    },
-    click() {
-      for (const handler of element.listeners.click || []) handler();
-    },
-  };
-  Object.defineProperty(element, 'innerHTML', {
-    get() {
-      return '';
-    },
-    set() {
-      element.children.length = 0;
-    },
-  });
-  return element;
-}
-
-const doc = { createElement: (tagName) => makeElement(tagName) };
-
-/** Text a chip renders, label + optional requirement badge. */
-const chipText = (chip) =>
-  chip.children.map((child) => child.textContent).join(' ');
+import { createMapSourceControls } from './ui/mapSourceControls.js';
 
 // Shaped exactly like MapStackController.getStacks() output.
 const CONTROLLER_STACKS = [
@@ -121,24 +57,36 @@ const CONTROLLER_STACKS = [
   },
 ];
 
-test('the row renders exactly the five owner-approved sources', () => {
-  const container = makeElement();
-  renderMapStackChips(container, CONTROLLER_STACKS, {
-    activeId: 'photoreal',
-    doc,
+/** The chips Map Source controls publish for `stacks`, as the dock renders them. */
+function publishedChips(stacks, activeId) {
+  const views = [];
+  createMapSourceControls({
+    controller: {
+      getStacks: () => stacks,
+      getActiveId: () => activeId,
+      getState: () => ({
+        activeId,
+        activeStack: stacks.find((stack) => stack.id === activeId),
+        status: 'ready',
+      }),
+      setStack: async () => null,
+    },
+    subscribe: () => () => {},
+    publish: (view) => views.push(view),
   });
+  return views.at(-1).chips;
+}
 
+test('the row presents exactly the five owner-approved sources', () => {
+  const chips = mapStackChipModels(CONTROLLER_STACKS, 'photoreal');
   assert.deepEqual(
-    container.children.map((chip) => chip.dataset.stackId),
+    chips.map((chip) => chip.id),
     ['photoreal', 'bing-aerial', 'bing-labels', 'esri-imagery', 'osm'],
   );
-  assert.deepEqual(container.children.map(chipText), [
-    'Google 3D',
-    'Bing Aerial',
-    'Bing Labels',
-    'Esri Satellite',
-    'OSM',
-  ]);
+  assert.deepEqual(
+    chips.map((chip) => chip.label),
+    ['Google 3D', 'Bing Aerial', 'Bing Labels', 'Esri Satellite', 'OSM'],
+  );
   assert.deepEqual(PRESENTED_MAP_STACK_IDS, [
     'photoreal',
     'bing-aerial',
@@ -146,102 +94,31 @@ test('the row renders exactly the five owner-approved sources', () => {
     'esri-imagery',
     'osm',
   ]);
-  assert.ok(
-    container.children.every(
-      (chip) => chip.tagName === 'button' && chip.type === 'button',
-    ),
-  );
-  assert.ok(
-    container.children.every((chip) =>
-      chip.classList.contains(MAP_STACK_CHIP_CLASS),
-    ),
-  );
 });
 
 test('internal and future stacks stay outside the approved presentation set', () => {
-  const container = makeElement();
   // A future Hybrid stack may land in the controller, but it must not appear
   // until the owner-approved presentation allowlist explicitly includes it.
   const withHybrid = [
     ...CONTROLLER_STACKS,
     { id: 'hybrid', label: 'Hybrid', available: true },
   ];
-  renderMapStackChips(container, withHybrid, { activeId: 'photoreal', doc });
+  const chips = mapStackChipModels(withHybrid, 'photoreal');
 
-  assert.equal(container.children.length, 5);
-  assert.doesNotMatch(container.children.map(chipText).join(' '), /Hybrid/);
+  assert.equal(chips.length, 5);
+  assert.ok(chips.every((chip) => chip.id !== 'hybrid'));
 });
 
-test('re-rendering replaces the previous chips instead of stacking a second row', () => {
-  const container = makeElement();
-  renderMapStackChips(container, CONTROLLER_STACKS, {
-    activeId: 'photoreal',
-    doc,
-  });
-  renderMapStackChips(container, CONTROLLER_STACKS, { activeId: 'osm', doc });
-
-  assert.equal(container.children.length, PRESENTED_MAP_STACK_IDS.length);
-});
-
-test('clicking a chip dispatches that stack id — the same selection the dropdown made', () => {
-  const container = makeElement();
-  const selected = [];
-  renderMapStackChips(container, CONTROLLER_STACKS, {
-    activeId: 'photoreal',
-    onSelect: (stackId) => selected.push(stackId),
-    doc,
-  });
-
-  container.children[4].click();
-  container.children[3].click();
-  container.children[1].click();
-  assert.deepEqual(selected, ['osm', 'esri-imagery', 'bing-aerial']);
-});
-
-test('the active chip is the pressed chip, and exactly one is pressed', () => {
-  const container = makeElement();
-  renderMapStackChips(container, CONTROLLER_STACKS, {
-    activeId: 'bing-labels',
-    doc,
-  });
-
-  const pressed = container.children.filter(
-    (chip) => chip.getAttribute('aria-pressed') === 'true',
-  );
+test('the active chip is the only active chip', () => {
   assert.deepEqual(
-    pressed.map((chip) => chip.dataset.stackId),
+    mapStackChipModels(CONTROLLER_STACKS, 'bing-labels')
+      .filter((chip) => chip.active)
+      .map((chip) => chip.id),
     ['bing-labels'],
   );
-  assert.ok(pressed[0].classList.contains('active'));
-  assert.ok(
-    container.children
-      .filter((chip) => chip.dataset.stackId !== 'bing-labels')
-      .every((chip) => !chip.classList.contains('active')),
-  );
 });
 
-test('the lit chip tracks controller state, not the click', () => {
-  const container = makeElement();
-  renderMapStackChips(container, CONTROLLER_STACKS, {
-    activeId: 'photoreal',
-    doc,
-  });
-
-  // A rejected/superseded switch reports the stack that is genuinely active.
-  syncMapStackChips(container, 'photoreal');
-  assert.ok(container.children[0].classList.contains('active'));
-  assert.equal(container.children[4].getAttribute('aria-pressed'), 'false');
-
-  // A landed switch moves both the class and the pressed state.
-  syncMapStackChips(container, 'osm');
-  assert.ok(container.children[4].classList.contains('active'));
-  assert.equal(container.children[4].getAttribute('aria-pressed'), 'true');
-  assert.ok(!container.children[0].classList.contains('active'));
-  assert.equal(container.children[0].getAttribute('aria-pressed'), 'false');
-});
-
-test('keyless ion stacks stay focusable, aria-disabled, and say why', () => {
-  const container = makeElement();
+test('keyless ion stacks are unavailable, badged, and say why', () => {
   const keyless = CONTROLLER_STACKS.map((stack) =>
     stack.requiresIon
       ? {
@@ -252,49 +129,26 @@ test('keyless ion stacks stay focusable, aria-disabled, and say why', () => {
         }
       : stack,
   );
-  const selected = [];
-  renderMapStackChips(container, keyless, {
-    activeId: 'photoreal',
-    onSelect: (stackId) => selected.push(stackId),
-    doc,
-  });
+  const chips = publishedChips(keyless, 'photoreal');
+  const bingAerial = chips[1];
 
-  const bingAerial = container.children[1];
+  assert.equal(bingAerial.available, false);
+  assert.equal(bingAerial.requirement, 'ION');
   assert.equal(
-    bingAerial.disabled,
-    false,
-    'unavailable sources remain keyboard-reachable',
-  );
-  assert.equal(bingAerial.getAttribute('aria-disabled'), 'true');
-  assert.equal(
-    bingAerial.getAttribute('aria-label'),
+    bingAerial.ariaLabel,
     'Bing Aerial unavailable: Needs CESIUM_ION_TOKEN — add it in Provider Settings',
   );
-  assert.ok(bingAerial.classList.contains('unavailable'));
   assert.equal(
     bingAerial.title,
     'Needs CESIUM_ION_TOKEN — add it in Provider Settings',
   );
-  assert.equal(chipText(bingAerial), 'Bing Aerial ION');
-
-  bingAerial.click();
-  assert.deepEqual(
-    selected,
-    [],
-    'an unavailable stack must not reach the switch path',
-  );
-
-  assert.equal(
-    container.children[3].getAttribute('aria-disabled'),
-    'false',
-    'OSM stays selectable',
-  );
+  assert.equal(chips[3].available, true, 'Esri stays selectable');
+  assert.equal(chips[3].ariaLabel, null);
 });
 
 test('a non-ion stack that fails never claims an ion token is required', () => {
   // The startup fallback-to-OSM case: Google 3D tiles failed to load, so
   // photoreal is unavailable for a reason that has nothing to do with ion.
-  const container = makeElement();
   const tilesFailed = CONTROLLER_STACKS.map((stack) =>
     stack.id === 'photoreal'
       ? {
@@ -305,28 +159,24 @@ test('a non-ion stack that fails never claims an ion token is required', () => {
         }
       : stack,
   );
-  renderMapStackChips(container, tilesFailed, { activeId: 'osm', doc });
+  const chips = publishedChips(tilesFailed, 'osm');
+  const google = chips[0];
 
-  const google = container.children[0];
-  assert.equal(google.getAttribute('aria-disabled'), 'true');
+  assert.equal(google.available, false);
   assert.equal(
-    google.getAttribute('aria-label'),
+    google.ariaLabel,
     'Google 3D unavailable: Needs GOOGLE_MAPS_API_KEY — add it in Provider Settings',
   );
   assert.equal(
-    chipText(google),
-    'Google 3D',
+    google.requirement,
+    '',
     'no ION badge on a stack that does not need ion',
   );
   assert.equal(
     google.title,
     'Needs GOOGLE_MAPS_API_KEY — add it in Provider Settings',
   );
-  assert.equal(
-    chipText(container.children[1]),
-    'Bing Aerial',
-    'available ion stacks stay unbadged',
-  );
+  assert.equal(chips[1].requirement, '', 'available ion stacks stay unbadged');
 });
 
 test("models carry the stack's own reason and never invent an active chip", () => {
@@ -381,15 +231,6 @@ test("models carry the stack's own reason and never invent an active chip", () =
   assert.deepEqual(mapStackChipModels(undefined, 'osm'), []);
 });
 
-test('a missing row or document is inert rather than throwing during boot', () => {
-  assert.deepEqual(renderMapStackChips(null, CONTROLLER_STACKS, { doc }), []);
-  assert.deepEqual(
-    renderMapStackChips(makeElement(), CONTROLLER_STACKS, { doc: {} }),
-    [],
-  );
-  assert.doesNotThrow(() => syncMapStackChips(null, 'osm'));
-});
-
 test('the active cyan survives hover', () => {
   const css = readStylesheet(new URL('../style.css', import.meta.url));
   const hover = css.indexOf('.map-stack-chip:hover');
diff --git a/src/ui/commandDock.test.mjs b/src/ui/commandDock.test.mjs
index 223d4a6d2..46b7c855b 100644
--- a/src/ui/commandDock.test.mjs
+++ b/src/ui/commandDock.test.mjs
@@ -127,6 +127,11 @@ test('the map source row renders the published chips and selects through the con
   );
   const [bing, esri, osm] = chips;
   assert.equal(bing.classList.contains('unavailable'), true);
+  assert.equal(
+    bing.disabled,
+    false,
+    'unavailable chips stay keyboard-reachable',
+  );
   assert.equal(bing.getAttribute('aria-disabled'), 'true');
   assert.equal(
     bing.getAttribute('aria-label'),
diff --git a/src/ui/mapSource.js b/src/ui/mapSource.js
index e6b1925af..796bbac66 100644
--- a/src/ui/mapSource.js
+++ b/src/ui/mapSource.js
@@ -1,9 +1,6 @@
 export { createMapSourceControls } from './mapSourceControls.js';
 export {
-  MAP_STACK_CHIP_CLASS,
   PRESENTED_MAP_STACK_IDS,
   mapStackChipModel,
   mapStackChipModels,
-  renderMapStackChips,
-  syncMapStackChips,
 } from '../mapStackChips.js';