diff --git a/changelog.d/1417-clear-all-saved-layers.md b/changelog.d/1417-clear-all-saved-layers.md new file mode 100644 index 000000000..8cffbd366 --- /dev/null +++ b/changelog.d/1417-clear-all-saved-layers.md @@ -0,0 +1,3 @@ +- Make Clear All clear the saved and shared layer selection. Layers that a + context mode had already switched off no longer stay in the share link and + come back on reload (samehkhamis, #1417). diff --git a/src/contextSessionOrdering.test.mjs b/src/contextSessionOrdering.test.mjs index 2f9bfeae0..d7127e8d5 100644 --- a/src/contextSessionOrdering.test.mjs +++ b/src/contextSessionOrdering.test.mjs @@ -94,6 +94,15 @@ test('context restore settles the Contacts coordinator before dependency fanout' assert.match(restore, /excludeLayerIds: settleContactsCoordinator[\s\S]*?contactsCoordinatorId/); }); +test('Clear All during the Contacts settle stops the snapshot coming back', () => { + const restore = _restoreContextSession.toString(); + const coordinatorOff = restore.indexOf('const coordinatorSettled = await this._dataManager.setEnabled('); + const cancelled = restore.indexOf('if (restoreState.cancelled) return;', coordinatorOff); + const fanout = restore.indexOf('await this._dataManager.restoreEnabledLayerIds(', coordinatorOff); + assert.ok(cancelled > coordinatorOff, 'the cancellation is checked after Contacts settles'); + assert.ok(fanout > cancelled, 'and before the snapshot fans out'); +}); + test('context restore preserves its exact pending target after a failed transition', () => { const restore = _restoreContextSession.toString(); assert.match(restore, /const replayError = await settleContextIntentReplay\(\{/); diff --git a/src/data/layerState.test.mjs b/src/data/layerState.test.mjs index ba5496227..d05fb9a95 100644 --- a/src/data/layerState.test.mjs +++ b/src/data/layerState.test.mjs @@ -2858,3 +2858,135 @@ test('explicit provider switches reach durable state without adopting unrelated coordinator.destroy(); } }); + +function enabledLayerParam(sink) { + return new URLSearchParams(encode(sink.provider())).get('l'); +} + +test('Clear All drops a saved layer a context mode had already switched off', async () => { + const manager = productionManager(); + const storage = memoryStorage(); + const sink = shareSink(); + const coordinator = new LayerStateCoordinator(manager, sink, { storage }); + await coordinator.start(); + await manager.setEnabled('earthquakes', true, { origin: 'user' }); + await manager.setEnabled('flights', true, { origin: 'user' }); + assert.deepEqual(coordinator.getDurableState().enabledLayerIds, [ + 'earthquakes', + 'flights', + ]); + // Context isolation is a temporary, passive takeover: the saved choice stays. + await manager.setEnabled('earthquakes', false, { origin: 'programmatic' }); + assert.deepEqual(coordinator.getDurableState().enabledLayerIds, [ + 'earthquakes', + 'flights', + ]); + + const updatesBefore = sink.updates; + const result = await manager.clearSelectedLayers({ origin: 'user' }); + assert.deepEqual(result.targetIds, ['flights']); + assert.ok(sink.updates > updatesBefore, 'the URL is told to refresh'); + assert.deepEqual([...manager.getEnabledLayerIds()], []); + assert.deepEqual(coordinator.getDurableState().enabledLayerIds, []); + assert.equal(enabledLayerParam(sink), '', 'the URL carries no layers'); + assert.deepEqual( + parseStoredLayerState(storage.getItem(LAYER_STATE_STORAGE_KEY)) + .enabledLayerIds, + [], + 'a reload restores nothing', + ); + coordinator.destroy(); +}); + +test('Clear All drops a saved layer a dependency release superseded mid-batch', async () => { + let manager; + manager = productionManager({ + // Contacts-style owner: its own teardown releases a borrowed dependency + // without awaiting it, which supersedes Clear All's turn for that layer. + 'military-awareness': { + disable() { + void manager.setEnabled('flights', false, { origin: 'programmatic' }); + return true; + }, + }, + }); + const storage = memoryStorage(); + const sink = shareSink(); + const coordinator = new LayerStateCoordinator(manager, sink, { storage }); + await coordinator.start(); + await manager.setEnabled('earthquakes', true, { origin: 'user' }); + await manager.setEnabled('flights', true, { origin: 'user' }); + await manager.setEnabled('military-awareness', true, { origin: 'user' }); + + const result = await manager.clearSelectedLayers({ origin: 'user' }); + assert.equal( + result.items.find((item) => item.id === 'flights')?.superseded, + true, + 'the fixture reproduces the superseded flights turn', + ); + assert.deepEqual([...manager.getEnabledLayerIds()], []); + assert.deepEqual(coordinator.getDurableState().enabledLayerIds, []); + assert.equal(enabledLayerParam(sink), ''); + coordinator.destroy(); +}); + +test('Clear All keeps a layer a newer explicit request turned back on', async () => { + const manager = productionManager(); + const sink = shareSink(); + const coordinator = new LayerStateCoordinator(manager, sink, { + storage: memoryStorage(), + }); + await coordinator.start(); + await manager.setEnabled('earthquakes', true, { origin: 'user' }); + await manager.setEnabled('flights', true, { origin: 'user' }); + const clearing = manager.clearSelectedLayers({ origin: 'user' }); + // A direct request issued during the batch owns its layer. + const reenable = manager.setEnabled('earthquakes', true, { origin: 'user' }); + await Promise.all([clearing, reenable]); + assert.deepEqual([...manager.getEnabledLayerIds()], ['earthquakes']); + assert.deepEqual(coordinator.getDurableState().enabledLayerIds, [ + 'earthquakes', + ]); + assert.equal(enabledLayerParam(sink), 'e'); + coordinator.destroy(); +}); + +test('a passive clear never rewrites saved layers', async () => { + const manager = productionManager(); + const storage = memoryStorage(); + const coordinator = new LayerStateCoordinator(manager, shareSink(), { + storage, + }); + await coordinator.start(); + await manager.setEnabled('flights', true, { origin: 'user' }); + await manager.setEnabled('flights', false, { origin: 'programmatic' }); + const writes = storage.writes.length; + await manager.clearSelectedLayers({ origin: 'programmatic' }); + assert.deepEqual(coordinator.getDurableState().enabledLayerIds, ['flights']); + assert.equal(storage.writes.length, writes); + coordinator.destroy(); +}); + +test('Clear All cancels a saved restore still waiting to start', async () => { + const gate = deferred(); + const manager = productionManager(); + const sink = shareSink(); + const coordinator = new LayerStateCoordinator(manager, sink, { + storage: memoryStorage(), + restoreGate: gate.promise, + }); + const params = new URLSearchParams('v=2&l=e.f'); + const restore = coordinator.start({ + shareLayerState: decodeLayerStateParams(params), + }); + await manager.clearSelectedLayers({ origin: 'user' }); + assert.deepEqual(coordinator.getDurableState().enabledLayerIds, []); + gate.resolve(); + const results = await restore; + for (const id of ['earthquakes', 'flights']) { + assert.equal(results.find((result) => result.layerId === id).succeeded, false); + } + assert.deepEqual([...manager.getEnabledLayerIds()], []); + assert.equal(enabledLayerParam(sink), ''); + coordinator.destroy(); +}); diff --git a/src/data/layerStateCoordinator.js b/src/data/layerStateCoordinator.js index ef3a4ea4a..40208831a 100644 --- a/src/data/layerStateCoordinator.js +++ b/src/data/layerStateCoordinator.js @@ -250,6 +250,30 @@ export class LayerStateCoordinator { this._revokePendingTrackingWatch('owner-layer-disabled'); } if (!isExplicitLayerStateOrigin(change.origin)) return; + if (change.type === 'selection-cleared') { + // Only layers a newer request turned back on survive Clear All. Any + // other saved layer would reappear from the share link or a reload. + const stillEnabled = new Set(change.enabledLayerIds || []); + const dropped = this._durableState.enabledLayerIds.filter( + (id) => !stillEnabled.has(id), + ); + if (dropped.length === 0) return; + for (const layerId of dropped) { + this._restoreControllers + .get(layerId) + ?.abort('superseded-by-explicit-visibility'); + } + if (dropped.some((id) => SHARE_TRACKING_RESTORE_POLICIES[id])) { + this._revokePendingTrackingWatch('superseded-by-explicit-visibility'); + } + this._commitExplicit({ + ...this._durableState, + enabledLayerIds: this._durableState.enabledLayerIds.filter((id) => + stillEnabled.has(id), + ), + }); + return; + } if (change.type === 'visibility') { this._restoreControllers .get(change.layerId) diff --git a/src/data/lifecycle.js b/src/data/lifecycle.js index c8946e7bb..6c2290c72 100644 --- a/src/data/lifecycle.js +++ b/src/data/lifecycle.js @@ -1650,6 +1650,15 @@ export class LayerLifecycle { } } } + // Clear All owns the whole selection, not only the layers it switched off + // itself. A layer already off for a passive reason (isolated by a context + // mode, released by a dependency owner, or superseded mid-batch) gets no + // OFF of its own here, so publish the set that is still on afterwards. + this._notifyListeners({ + type: 'selection-cleared', + origin, + enabledLayerIds: [...this.getEnabledLayerIds()], + }); const items = targetIds.map((id) => { const state = this.getLayerLifecycleState(id) || { enabled: false, diff --git a/src/ui/contextSession.js b/src/ui/contextSession.js index 241b5f168..936fc2cb1 100644 --- a/src/ui/contextSession.js +++ b/src/ui/contextSession.js @@ -86,6 +86,9 @@ export async function _restoreContextSession({ throw error; } } + // Clear All may have run while Contacts settled. It owns the selection + // now, so the snapshot must not come back behind it. + if (restoreState.cancelled) return; await this._dataManager.restoreEnabledLayerIds( restoreState.enabledLayerIds, {