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.
This commit is contained in:
Sameh Khamis committed 2026-10-31 19:28:02 -07:00
1 parent 1de4f635ca
commit 0b8cb4f33f
6 files changed
+71 -326

No files matched your search

+2 -2
View File
@@ -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
+3 -3
View File
@@ -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
+9 -107
View File
@@ -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));
}
}
+52 -211
View File
@@ -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');
+5
View File
@@ -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'),
-3
View File
@@ -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';