mirror of
https://github.com/bilawalsidhu/gods-eye-view.git
synced 2026-11-04 18:34:02 +00:00
fix(layers): make Clear All clear the saved and shared layer selection (#1417)
* fix(layers): make Clear All clear the saved and shared layer selection After Clear All every layer control was off, but the share URL could keep listing layers (for example l=e.f) and a reload switched them back on. The URL and local preferences are written from the committed layer state, which only explicit visibility changes update. Clear All issued an explicit OFF only for layers it found on at the start of the batch and whose turn was not superseded. A layer already off for a passive reason (isolated by Contacts or another context mode, or released by Contacts' own teardown while the batch ran) never received one, so it stayed in the committed state. Clear All also discards the context snapshot that would have restored those layers, leaving the screen and the URL out of step. Clear All now publishes the set of layers still on once its batch settles, and the layer-state coordinator commits that selection: every saved layer not in it is dropped and its passive restore is cancelled, while a layer a newer explicit request turned back on is kept. A context restore interrupted by Clear All no longer brings its snapshot back after Contacts settles. * docs: changelog for #1417
This commit is contained in:
1 parent
b7ece1d4d1
commit
fefd5cdcf1
6 files changed
+180
No files matched your search
@@ -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\(\{/);
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
@@ -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)
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
{
|
||||
|
||||
Reference in new issue
Block a user