From e348786adb55184ab13a6db4357f69da3fdf0779 Mon Sep 17 00:00:00 2001 From: Jeff Maki Date: Fri, 30 Oct 2026 13:04:59 -0400 Subject: [PATCH] Never auto-pick custom imagery, and keep a custom pick's template Custom imagery declares no coverage, so it counted as worldwide and followed the user into workspaces it doesn't cover. Only a choice can show it now. Rapid has one custom slot, so a saved custom pick keeps its own template. Document the imagery order in CLAUDE.md. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 19 +++++++++++ test/unit/util/rapid-background.test.ts | 43 ++++++++++++++++++++++++- util/rapid-background.ts | 25 ++++++++++---- 3 files changed, 80 insertions(+), 7 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 919d88b..7f9924e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -85,6 +85,25 @@ intended behavior. Tests are generated **to the comment (the spec), not the current code** — if the code diverges, the test is left RED to document the bug. Don't soften assertions to make buggy code pass. +## Background imagery + +When a workspace opens in Rapid, [util/rapid-background.ts](util/rapid-background.ts) +chooses its background, in this order. Keep to it when changing imagery selection: + +1. **`background=` in the URL**, if it covers the center of the view (a custom + template in the URL is always honored, since it declares no coverage). +2. **The user's saved pick for this workspace**, if it covers the center of the + view. Partial coverage is fine because they chose it. A pick that doesn't + cover the workspace resets to step 3. Picks are per workspace, in the + browser's local storage, recorded only from Rapid's Background pane; a custom + pick saves its own template, because Rapid has one custom slot. +3. **Our pick, only from sources covering the whole extent** (sampled at nine + points across the bbox; worldwide sources qualify): the current imagery, then + the area's best, then the first. Never custom imagery, since we can't know + what it covers. +4. **If nothing covers the whole extent**, what covers the center of the view, + rather than no imagery. + ## Gotchas (learned the hard way) - **OSM URLs include `/api/0.6/`.** The OSM client base is diff --git a/test/unit/util/rapid-background.test.ts b/test/unit/util/rapid-background.test.ts index 9d8b863..b5c7fa1 100644 --- a/test/unit/util/rapid-background.test.ts +++ b/test/unit/util/rapid-background.test.ts @@ -12,6 +12,9 @@ const kingCounty: RapidBackgroundSource = { id: 'King_County', template: 'https: * An imagery system covering `here` and showing `current`. Like Rapid's, * `visibleSources` always includes the current imagery. */ +/** Rapid's one custom imagery slot. */ +const customSlot: RapidBackgroundSource = { id: 'custom', template: '' }; + function fakeImagery(here: RapidBackgroundSource[], current: RapidBackgroundSource | null): RapidBackgroundImagery { const imagery: RapidBackgroundImagery = { _baseLayer: current, @@ -22,7 +25,8 @@ function fakeImagery(here: RapidBackgroundSource[], current: RapidBackgroundSour baseLayerSource: vi.fn((source: RapidBackgroundSource) => { imagery._baseLayer = source; }), - setSourceByID: vi.fn() + setSourceByID: vi.fn(), + getSourceByID: (id: string) => (id === 'custom' ? customSlot : undefined) }; return imagery; } @@ -289,6 +293,43 @@ describe('choosing the background in order', () => { expect(setItem).toHaveBeenCalledTimes(2); }); + it('resets custom imagery on a workspace nobody chose it for', async () => { + getWorkspaceBbox.mockResolvedValue(extent); + const custom = { id: 'custom', template: 'https://mine/{z}', props: {} }; + const imagery = fakeImagery([custom, worldwide], custom); + + await choose(coverageContext(imagery)); + + expect(imagery.baseLayerSource).toHaveBeenCalledWith(worldwide); + }); + + it('resets a pick from another workspace when it doesn\'t cover this one', async () => { + window.localStorage.setItem('workspace-background:8', 'Downtown'); + getWorkspaceBbox.mockResolvedValue(extent); + const imagery = fakeImagery([partial, worldwide], partial); + + await choose(coverageContext(imagery)); // workspace 7 + + expect(imagery.baseLayerSource).toHaveBeenCalledWith(worldwide); + }); + + it('keeps a custom pick\'s own template for its workspace', async () => { + getWorkspaceBbox.mockResolvedValue(extent); + customSlot.template = 'https://first/{z}'; + const imagery = fakeImagery([worldwide], worldwide); + const context = coverageContext(imagery, { settings: { set: vi.fn() } }); + await choose(context); + + context.systems.settings!.set('imagery.lastUsed', 'custom'); + expect(window.localStorage.getItem('workspace-background:7')).toBe('custom:https://first/{z}'); + + customSlot.template = 'https://other-workspace/{z}'; + await choose(context); + + expect(customSlot.template).toBe('https://first/{z}'); + expect(imagery.baseLayerSource).toHaveBeenCalledWith(customSlot); + }); + it('doesn\'t treat our own pick as the user\'s', async () => { getWorkspaceBbox.mockResolvedValue(extent); const imagery = fakeImagery([partial, whole], partial); diff --git a/util/rapid-background.ts b/util/rapid-background.ts index 5963415..3391510 100644 --- a/util/rapid-background.ts +++ b/util/rapid-background.ts @@ -25,6 +25,7 @@ export interface RapidBackgroundImagery { baseLayerSource(source: RapidBackgroundSource): unknown; /** Also handles Esri Wayback's dated IDs. */ setSourceByID(id: string): void; + getSourceByID(id: string): RapidBackgroundSource | undefined; /** Rapid 3's coverage index. */ _whichPolygon?: CoverageQuery | null; /** Rapid 2's coverage index, and the coverage areas by lowercase source ID. */ @@ -93,11 +94,12 @@ function backgroundsHere(imagery: RapidBackgroundImagery): RapidBackgroundSource * The background imagery we'd pick where the map is now: the current imagery if * it covers this place, else the imagery marked best here, else the first that * covers it. With an `extent`, imagery covering all of it comes first; only if - * none does, what covers the center of the view. + * none does, what covers the center of the view. Never custom imagery: it + * declares no coverage, so only someone choosing it can know it fits. */ export function chooseBackground(imagery: RapidBackgroundImagery, extent?: BoundingBox): RapidBackgroundSource | null { const current = imagery._baseLayer; - const here = backgroundsHere(imagery); + const here = backgroundsHere(imagery).filter(s => s.id !== 'custom'); const covering = extent ? here.filter(s => coversExtent(imagery, s, extent)) : here; const backgrounds = covering.length ? covering : here; @@ -127,8 +129,10 @@ function savedPick(workspaceId: number | undefined): string | undefined { function rememberPicks(context: RapidBackgroundContext) { const save = (id: unknown) => { if (context.workspaceId === undefined || typeof id !== 'string') return; + // Rapid has one custom slot, so a custom pick keeps its own template. + const pick = id === 'custom' ? `custom:${context.systems.imagery.getSourceByID('custom')?.template ?? ''}` : id; try { - window.localStorage.setItem(pickKey(context.workspaceId), id); + window.localStorage.setItem(pickKey(context.workspaceId), pick); } catch { /* private browsing: the pick just won't stick */ } }; @@ -166,13 +170,22 @@ function urlBackground(): string | undefined { */ function applyChosen(context: RapidBackgroundContext): boolean { const imagery = context.systems.imagery; - const chosen = [urlBackground(), savedPick(context.workspaceId)].filter(Boolean) as string[]; + const fromUrl = urlBackground(); + // Rapid has already applied a custom template from the URL. + if (fromUrl?.startsWith('custom:')) return true; + + const chosen = [fromUrl, savedPick(context.workspaceId)].filter(Boolean) as string[]; if (!chosen.length) return false; const here = backgroundsHere(imagery); for (const id of chosen) { - // Rapid has already applied a custom template from the URL. - if (id.startsWith('custom:')) return true; + if (id.startsWith('custom:')) { + const custom = imagery.getSourceByID('custom'); + if (!custom) continue; + custom.template = id.slice('custom:'.length); + imagery.baseLayerSource(custom); + return true; + } const source = here.find(s => s.id === id || id.startsWith(`${s.id}_`)); // e.g. EsriWayback_2024-01-01 if (id === 'none' || source) { if (id !== imagery._baseLayer?.id) imagery.setSourceByID(id);