Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .changeset/clear-all-models-selection-reset.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@ifc-lite/viewer': patch
---

Fix `clearAllModels()` leaving the `EntityRef`-keyed half of selection state (`selectedEntity`, `selectedEntities`, `selectedEntitiesSet`, `selectedModelId`, `activeStorey`) pointing at models that were just removed.

The `all-models-cleared` teardown scope only ever cleared the global-id half (`selectedEntityId`, `selectedEntityIds`, `selectedStoreys`). `resetViewerState()` clears both halves, so the gap only showed on a path that calls `clearAllModels()` without it — `GeoreferencingPanel.tsx`'s `reloadModelsForAlignment`, which left the properties panel bound to a model that no longer existed.

Since `clearAllModels` removes every model, there is no surviving federated sibling to preserve a selection for (unlike the single-model `model-removed` scope, which filters by `modelId`), so both halves now clear unconditionally.
62 changes: 62 additions & 0 deletions apps/viewer/src/store/clearAllModels-selection-stale.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
/* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at https://mozilla.org/MPL/2.0/. */

/**
* `clearAllModels()` (the `all-models-cleared` teardown scope) clears the
* global-id half of selection state (`selectedEntityId`, `selectedEntityIds`,
* `selectedStoreys`) but left the `EntityRef`-keyed half (`selectedEntity`,
* `selectedEntities`, `selectedEntitiesSet`, `selectedModelId`,
* `activeStorey`) untouched.
*
* `resetViewerState()` clears both halves, so the asymmetry only shows on a
* path that calls `clearAllModels()` without `resetViewerState()` —
* `GeoreferencingPanel.tsx`'s `reloadModelsForAlignment`, same shape as
* `clearAllModels-overlay-stale.test.ts` (#2854) and
* `removeModel-compare-stale.test.ts`.
*
* #3348.
*/

import { describe, it } from 'node:test';
import assert from 'node:assert';
import { useViewerStore } from './index.js';
import type { FederatedModel } from './types.js';

function model(id: string, idOffset: number, maxExpressId: number): FederatedModel {
return { id, name: id, visible: true, idOffset, maxExpressId } as unknown as FederatedModel;
}

describe('clearAllModels drops the EntityRef-keyed half of selection, not just the global-id half', () => {
it('leaves nothing pointing at a removed model after clearAllModels()', () => {
useViewerStore.setState({
models: new Map([['A', model('A', 0, 100)]]),
activeModelId: 'A',
});

// A distinctly non-default, clearly-stale ref: model 'A' is the only
// model in the store and gets removed by clearAllModels(), so if any of
// this survives it can only be because the field was never cleared.
useViewerStore.getState().setSelectedEntity({ modelId: 'A', expressId: 42 });
useViewerStore.getState().setSelectedEntities([{ modelId: 'A', expressId: 42 }]);
useViewerStore.getState().addEntityToSelection({ modelId: 'A', expressId: 42 });
useViewerStore.setState({ selectedModelId: 'A' });
useViewerStore.getState().setActiveStorey({ modelId: 'A', expressId: 44 });

const before = useViewerStore.getState();
assert.notStrictEqual(before.selectedEntity, null, 'precondition: an entity is selected');
assert.strictEqual(before.selectedEntities.length, 1, 'precondition: selectedEntities is populated');
assert.strictEqual(before.selectedEntitiesSet.size, 1, 'precondition: selectedEntitiesSet is populated');
assert.strictEqual(before.selectedModelId, 'A', 'precondition: selectedModelId is set');
assert.notStrictEqual(before.activeStorey, null, 'precondition: activeStorey is set');

useViewerStore.getState().clearAllModels();

const after = useViewerStore.getState();
assert.strictEqual(after.selectedEntity, null, 'selectedEntity must not point at a removed model');
assert.deepStrictEqual(after.selectedEntities, [], 'selectedEntities must not list a removed model');
assert.strictEqual(after.selectedEntitiesSet.size, 0, 'selectedEntitiesSet must not key a removed model');
assert.strictEqual(after.selectedModelId, null, 'selectedModelId must not name a removed model');
assert.strictEqual(after.activeStorey, null, 'activeStorey must not point into a removed model');
});
});
22 changes: 16 additions & 6 deletions apps/viewer/src/store/slices/selectionSlice.teardown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,16 +57,26 @@ export const selectionTeardown = defineSliceTeardown(
}

if (scope.kind === 'all-models-cleared') {
// Only the global-id half, which is what `clearAllModels` has always
// written. The `EntityRef`-keyed half is deliberately absent: adding it
// here would be a behaviour change, not a restructuring, and this
// refactor is not the place to make one. (It is a real asymmetry —
// `resetViewerState` clears both halves — and is called out as a
// follow-up rather than smuggled in.)
// `clearAllModels` removes EVERY model, so — unlike `model-removed`,
// which must filter the `EntityRef`-keyed half by `modelId` to spare a
// surviving federated sibling's selection — there is no survivor to
// preserve anything for. Both halves clear unconditionally, same as
// `session-reset` above: the previous version of this arm wrote only
// the global-id half, which left `selectedEntity`, `selectedEntities`,
// `selectedEntitiesSet`, `selectedModelId` and `activeStorey` pointing
// at removed models (#3348) — `resetViewerState` already clears both
// halves, so this was purely a gap in `clearAllModels`'s own path
// (`GeoreferencingPanel.tsx`'s `reloadModelsForAlignment`, which calls
// `clearAllModels()` without `resetViewerState()`).
return {
selectedEntityId: null,
selectedEntityIds: new Set<number>(),
selectedStoreys: new Set<number>(),
activeStorey: null,
selectedEntity: null,
selectedEntitiesSet: new Set<string>(),
selectedEntities: [],
selectedModelId: null,
};
}

Expand Down
9 changes: 5 additions & 4 deletions apps/viewer/src/store/teardown-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,11 @@ const PINNED_SESSION_RESET_KEYS: readonly string[] = [

/** The same, for `all-models-cleared`. */
const PINNED_ALL_MODELS_CLEARED_KEYS: readonly string[] = [
'activeModelId', 'addElementModelId', 'addElementStoreyId', 'classFilter', 'geometryResult',
'ghostExceptEntities', 'hiddenEntities', 'hiddenEntitiesByModel', 'hierarchyBasketSelection',
'ifcDataStore', 'isolatedEntities', 'isolatedEntitiesByModel', 'meshColorBackup', 'models',
'pinboardEntities', 'selectedEntityId', 'selectedEntityIds', 'selectedStoreys',
'activeModelId', 'activeStorey', 'addElementModelId', 'addElementStoreyId', 'classFilter',
'geometryResult', 'ghostExceptEntities', 'hiddenEntities', 'hiddenEntitiesByModel',
'hierarchyBasketSelection', 'ifcDataStore', 'isolatedEntities', 'isolatedEntitiesByModel',
'meshColorBackup', 'models', 'pinboardEntities', 'selectedEntities', 'selectedEntitiesSet',
'selectedEntity', 'selectedEntityId', 'selectedEntityIds', 'selectedModelId', 'selectedStoreys',
];

/**
Expand Down
Loading