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
39 changes: 39 additions & 0 deletions apps/viewer/src/components/viewer/DxfUnderlayPanel.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -192,3 +192,42 @@ describe('DxfUnderlayPanel: independent 2D/3D visibility toggles (issue #2043)',
assert.equal(entry?.visible3D, false, '2D toggle must NOT touch visible3D');
});
});

describe('DxfUnderlayPanel: surfacing skipped entity types', () => {
beforeEach(() => {
useViewerStore.setState({ dxfUnderlays: [] });
});

afterEach(() => {
for (const { root, container } of mounted.splice(0)) {
act(() => {
root.unmount();
});
container.remove();
}
useViewerStore.setState({ dxfUnderlays: [] });
});

it('an underlay with skipped entity types shows a "Not imported" notice naming type and count', () => {
const underlay = emptyUnderlay('site.dxf');
underlay.skipped = { WIPEOUT: 40, HATCH: 2 };
useViewerStore.setState({ dxfUnderlays: [underlayState({ id: 'u1', underlay })] });
const container = renderPanel(false);
assert.ok(
container.textContent?.includes('Not imported'),
'expected a "Not imported" notice for skipped entity types',
);
assert.ok(container.textContent?.includes('40× WIPEOUT'), 'expected the WIPEOUT count to render');
assert.ok(container.textContent?.includes('2× HATCH'), 'expected the HATCH count to render');
});

it('an underlay with no skipped entity types shows no "Not imported" notice', () => {
const underlay = emptyUnderlay('clean.dxf');
useViewerStore.setState({ dxfUnderlays: [underlayState({ id: 'u1', underlay })] });
const container = renderPanel(false);
assert.ok(
!container.textContent?.includes('Not imported'),
'expected no "Not imported" notice when nothing was skipped',
);
});
});
19 changes: 19 additions & 0 deletions apps/viewer/src/components/viewer/DxfUnderlayPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,25 @@ function UnderlayCard({
</div>
)}

{/* Skipped entity types (parser-computed `underlay.skipped`, a
type→count map — distinct from `warnings`, a message list, so it
gets its own line rather than being concatenated with warnings
above). This is the only place a user can learn that part of the
DXF did not import: `ingestDxfFile` only logs `skipped` to the
console, and only when the underlay has ZERO drawable entities —
the common case of "most of it imported, N entities of type X
did not" was previously silent. */}
{Object.keys(underlay.skipped).length > 0 && (
<div className="flex items-start gap-1 text-[10px] text-amber-600 dark:text-amber-500 px-1">
<AlertTriangle className="h-3 w-3 mt-px shrink-0" />
<span>
Not imported: {Object.entries(underlay.skipped)
.map(([type, count]) => `${count}× ${type}`)
.join(', ')}
</span>
</div>
)}

{/* Opacity — PR #2114 review: the slider only affects the 2D drawing
panel. The 3D viewport's line pipeline (`Section2DOverlayRenderer`)
shares one un-blended `linePipeline`/uniform colour across the
Expand Down
29 changes: 23 additions & 6 deletions apps/viewer/src/components/viewer/selectionHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,18 @@ export async function handleSelectionClick(ctx: MouseHandlerContext, e: MouseEve
// Mention opening reassignment in the toast only when it
// happened — silence is preferable to "0 openings moved"
// for a wall with no doors / windows.
const op = wallTry.openings;
const opSummary =
op.toLeft + op.toRight > 0
? ` (${op.toLeft + op.toRight} opening${op.toLeft + op.toRight === 1 ? '' : 's'} reassigned)`
: '';
toast.success(`Wall split${opSummary} — Ctrl+Z to undo`);
toast.success(`Wall split${formatOpeningReassignSuffix(wallTry.openings)} — Ctrl+Z to undo`);
// `openings.skipped` (wall-opening-reassign.ts) counts doors/windows
// whose placement we couldn't interpret — they stay attached to the
// now-tombstoned source wall rather than either half, so they can
// end up orphaned. Computed by `reassignWallOpenings` on every
// split; previously only `toLeft`/`toRight` ever reached this
// toast, so a skip was silent.
if (wallTry.openings.skipped > 0) {
toast.info(
`${wallTry.openings.skipped} opening${wallTry.openings.skipped === 1 ? '' : 's'} could not be reassigned and may need manual repositioning`,
);
}
return;
}
const linearTry = state.splitLinearElementAtDistance(
Expand Down Expand Up @@ -952,6 +958,17 @@ function capitalize(s: string): string {
return s.charAt(0).toUpperCase() + s.slice(1);
}

/**
* The "(N openings reassigned)" suffix for the wall-split success toast.
* Pure so the wording is unit-testable without driving the full split flow
* through the store. Deliberately silent when nothing moved — see the
* call site's comment.
*/
export function formatOpeningReassignSuffix(op: { toLeft: number; toRight: number; skipped: number }): string {
const moved = op.toLeft + op.toRight;
return moved > 0 ? ` (${moved} opening${moved === 1 ? '' : 's'} reassigned)` : '';
}

/** Signed 2D polygon area via the shoelace formula. */
function polygonArea2D(points: Array<[number, number]>): number {
if (points.length < 3) return 0;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
/* 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/. */

/**
* `reassignWallOpenings` (wall-opening-reassign.ts) computes a `skipped`
* count whenever a door/window's opening has a placement it can't
* interpret — those openings are left attached to the wall segment that
* `splitWallAtDistance` is about to tombstone, so they can end up
* orphaned. The doc on `OpeningReassignSummary.skipped` says this exists
* "so the caller can surface a warning toast", but the wall-split click
* handler in `selectionHandlers.ts` only ever read `toLeft`/`toRight` —
* `skipped` reached the toast call site and was discarded.
*
* These tests exercise `formatOpeningReassignSuffix` (the pure formatter
* pulled out of the toast call) directly, and drive the real
* `handleSelectionClick` split branch through the store to confirm the
* `toast.info(...)` skipped-openings notice actually fires when
* `openings.skipped > 0`, and does not fire when it's 0.
*/

import '@/test/setup-dom.js';
import { describe, it, beforeEach, afterEach } from 'node:test';
import assert from 'node:assert/strict';
import { useViewerStore } from '@/store';
import { toast } from '@/components/ui/toast';
import { handleSelectionClick, formatOpeningReassignSuffix } from './selectionHandlers.js';
import type { MouseHandlerContext } from './mouseHandlerTypes.js';

function fakeCtx(): MouseHandlerContext {
const canvas = document.createElement('canvas');
return {
canvas,
renderer: {},
mouseState: { isDragging: false, isPanning: false, lastX: 0, lastY: 0, button: 0, startX: 0, startY: 0, didDrag: false },
activeToolRef: { current: 'split' },
} as unknown as MouseHandlerContext;
}

function fakeClick(): MouseEvent {
return { clientX: 0, clientY: 0 } as MouseEvent;
}

describe('formatOpeningReassignSuffix (pure)', () => {
it('renders a count when openings moved', () => {
assert.equal(formatOpeningReassignSuffix({ toLeft: 1, toRight: 2, skipped: 0 }), ' (3 openings reassigned)');
assert.equal(formatOpeningReassignSuffix({ toLeft: 1, toRight: 0, skipped: 0 }), ' (1 opening reassigned)');
});

it('renders nothing when no openings moved', () => {
assert.equal(formatOpeningReassignSuffix({ toLeft: 0, toRight: 0, skipped: 0 }), '');
assert.equal(formatOpeningReassignSuffix({ toLeft: 0, toRight: 0, skipped: 3 }), '');
});
});

describe('wall split toast: skipped openings notice', () => {
const originalSplit = useViewerStore.getState().splitWallAtDistance;
const originalInfo = toast.info;
const originalSuccess = toast.success;
let infoCalls: string[];
let successCalls: string[];

beforeEach(() => {
infoCalls = [];
successCalls = [];
(toast as { info: (m: string) => void }).info = (m: string) => infoCalls.push(m);
(toast as { success: (m: string) => void }).success = (m: string) => successCalls.push(m);
useViewerStore.setState({
splitTargetModelId: 'm1',
splitTargetExpressId: 42,
splitHoverDistance: 1.5,
splitMode: undefined,
slabCutAnchor: null,
clearSplitHover: () => {},
setSelectedEntityId: () => {},
} as Partial<ReturnType<typeof useViewerStore.getState>>);
});

afterEach(() => {
(toast as { info: (m: string) => void }).info = originalInfo;
(toast as { success: (m: string) => void }).success = originalSuccess;
useViewerStore.setState({ splitWallAtDistance: originalSplit });
});

it('surfaces a toast when the split left openings unreassigned', async () => {
useViewerStore.setState({
splitWallAtDistance: () => ({
ok: true,
left: { expressId: 1, globalId: 101 },
right: { expressId: 2, globalId: 102 },
openings: { toLeft: 1, toRight: 0, skipped: 2 },
}),
} as Partial<ReturnType<typeof useViewerStore.getState>>);

await handleSelectionClick(fakeCtx(), fakeClick());

assert.equal(successCalls.length, 1, 'expected the wall-split success toast');
assert.ok(
infoCalls.some((m) => m.includes('2 openings could not be reassigned')),
`expected a skipped-openings notice, got: ${JSON.stringify(infoCalls)}`,
);
});

it('shows no skipped-openings notice when nothing was skipped', async () => {
useViewerStore.setState({
splitWallAtDistance: () => ({
ok: true,
left: { expressId: 1, globalId: 101 },
right: { expressId: 2, globalId: 102 },
openings: { toLeft: 1, toRight: 1, skipped: 0 },
}),
} as Partial<ReturnType<typeof useViewerStore.getState>>);

await handleSelectionClick(fakeCtx(), fakeClick());

assert.equal(successCalls.length, 1, 'expected the wall-split success toast');
assert.equal(infoCalls.length, 0, 'expected no skipped-openings notice when skipped === 0');
});
});
Loading