fix(viewer): surface the skipped-openings notice on the numeric wall-split path too

.changeset/wall-split-notices-one-emitter.md

@@ -0,0 +1,9 @@
+---
+"@ifc-lite/viewer": patch
+---
+
+Report openings a wall split could not reassign on the typed-distance path too, not only the click path.
+
+A wall split commits from two places, and both call the same `MutationSlice.splitWallAtDistance`, so both receive the same `openings.skipped` count — openings that stay attached to the source wall the split has just tombstoned rather than moving to either half, and can therefore end up orphaned. #3023 taught only the canvas click handler (`selectionHandlers.ts`) to surface that count. The Split tool's numeric-distance panel (`tools/SplitNumericInput.tsx`) kept its own inlined copy of the "(N openings reassigned)" wording, read only `toLeft`/`toRight`, and never looked at `skipped` at all — so committing the identical split by typing a distance instead of clicking silently dropped the warning that clicking showed.
+
+Both notices now come from a single emitter, `notifyWallSplit` in the new `wallSplitNotice.ts`, which both call sites invoke instead of composing toasts themselves. An emitter rather than a shared formatter is the point: a formatter is still something a call site can neglect to call, which is exactly how these two paths came apart. The module imports nothing but the toast surface, so announcing a split does not drag `selectionHandlers.ts`'s store, geometry and measurement imports into the panel. Both paths are now pinned by tests asserting the full toast strings, in both directions — the warning when `skipped > 0`, and silence when it is 0.

apps/viewer/src/components/viewer/selectionHandlers.ts

@@ -13,6 +13,7 @@ import { useViewerStore } from '@/store';
 import { fromGlobalIdFromModels, toGlobalIdFromModels } from '@/store/globalId';
 import { pointInPolygon } from '@/lib/polygon-clip';
 import { toast } from '@/components/ui/toast';
+import { notifyWallSplit } from './wallSplitNotice.js';
 import { raycastForPolylinePoint, isNearPolylineStart,
   isDuplicateClickPoint,
 } from './measureHandlers.js';
@@ -158,21 +159,13 @@ export async function handleSelectionClick(ctx: MouseHandlerContext, e: MouseEve
       if (wallTry.ok) {
         state.clearSplitHover();
         state.setSelectedEntityId(wallTry.right.globalId);
-        // Mention opening reassignment in the toast only when it
-        // happened — silence is preferable to "0 openings moved"
-        // for a wall with no doors / windows.
-        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`,
-          );
-        }
+        // Both wall-split commit paths — here and the Split tool's
+        // numeric-distance panel — announce the split through the same
+        // emitter (`wallSplitNotice.ts`), so a notice added to one cannot
+        // go missing from the other. That is exactly how `openings.skipped`
+        // stayed silent on the typed-distance path after #3023 taught this
+        // one to report it.
+        notifyWallSplit(wallTry.openings);
         return;
       }
       const linearTry = state.splitLinearElementAtDistance(
@@ -958,17 +951,6 @@ 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;

apps/viewer/src/components/viewer/selectionHandlers.wallSplitToast.test.ts

@@ -12,19 +12,23 @@
  * 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
+ * These tests exercise `formatOpeningReassignSuffix` (the pure formatter,
+ * now in `wallSplitNotice.ts` beside `notifyWallSplit` — the single emitter
+ * both wall-split commit paths 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.
+ * `openings.skipped > 0`, and does not fire when it's 0. The other commit
+ * path — the Split tool's numeric-distance panel — is pinned against the
+ * same strings by `tools/SplitNumericInput.wallSplitToast.test.tsx`.
  */
 
 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 { handleSelectionClick } from './selectionHandlers.js';
+import { formatOpeningReassignSuffix } from './wallSplitNotice.js';
 import type { MouseHandlerContext } from './mouseHandlerTypes.js';
 
 function fakeCtx(): MouseHandlerContext {
@@ -94,7 +98,10 @@ describe('wall split toast: skipped openings notice', () => {
 
     await handleSelectionClick(fakeCtx(), fakeClick());
 
-    assert.equal(successCalls.length, 1, 'expected the wall-split success toast');
+    // The exact string, not a substring: `Wall split` and `— Ctrl+Z to undo`
+    // are what the numeric path's test pins too, so a change to either
+    // path's wording has to be a deliberate change to both.
+    assert.deepEqual(successCalls, ['Wall split (1 opening reassigned) — Ctrl+Z to undo']);
     assert.ok(
       infoCalls.some((m) => m.includes('2 openings could not be reassigned')),
       `expected a skipped-openings notice, got: ${JSON.stringify(infoCalls)}`,
@@ -113,7 +120,7 @@ describe('wall split toast: skipped openings notice', () => {
 
     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');
+    assert.deepEqual(successCalls, ['Wall split (2 openings reassigned) — Ctrl+Z to undo']);
+    assert.deepEqual(infoCalls, [], 'expected no skipped-openings notice when skipped === 0');
   });
 });

apps/viewer/src/components/viewer/tools/SplitNumericInput.tsx

@@ -33,6 +33,7 @@
 import { useEffect, useRef, useState, type ChangeEvent, type KeyboardEvent } from 'react';
 import { useViewerStore } from '@/store';
 import { toast } from '@/components/ui/toast';
+import { notifyWallSplit } from '../wallSplitNotice.js';
 
 const ACCENT = '#a855f7'; // purple-500
 const PANEL_OFFSET_PX = 32;
@@ -88,12 +89,11 @@ export function SplitNumericInput() {
     if (wallTry.ok) {
       clearSplitHover();
       setSelectedEntityId(wallTry.right.globalId);
-      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`);
+      // Same notices as the canvas click path — a split committed by typing
+      // a distance is the same edit on the same `splitWallAtDistance` result,
+      // including the warning when openings could not be reassigned
+      // (`openings.skipped`), which this path dropped until #3074.
+      notifyWallSplit(wallTry.openings);
       return;
     }
     const linearTry = splitLinearElementAtDistance(splitTargetModelId, splitTargetExpressId, distance);

apps/viewer/src/components/viewer/tools/SplitNumericInput.wallSplitToast.test.tsx

@@ -0,0 +1,108 @@
+/* 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/. */
+
+/**
+ * A wall split can be committed from TWO places: the canvas click handler
+ * (`selectionHandlers.ts`, covered by `selectionHandlers.wallSplitToast.test.ts`)
+ * and this panel's Cut button / Enter key. Both call the same
+ * `MutationSlice.splitWallAtDistance`, so both see the same `openings.skipped`
+ * count — openings that stay attached to the source wall the split has just
+ * tombstoned, and can end up orphaned.
+ *
+ * #3023 taught only the click handler to surface that count, leaving this panel
+ * with its own inlined copy of the success wording and no warning at all, so
+ * typing a distance instead of clicking hid a data problem that clicking
+ * reported. These tests drive the real panel through the real store and assert
+ * the EXACT strings the click path's test asserts, so the two commit paths
+ * cannot come apart again: against the pre-fix panel `toast.info` is never
+ * called.
+ */
+
+import '@/test/setup-dom.js';
+import { afterEach, beforeEach, describe, it } from 'node:test';
+import assert from 'node:assert/strict';
+import { useViewerStore } from '@/store';
+import { toast } from '@/components/ui/toast';
+import { render, cleanup, click } from '@/test/render';
+import { SplitNumericInput } from './SplitNumericInput.js';
+
+/** The panel's commit control. Labelled "Cut" — the Enter key runs the same `commitAt`. */
+function cutButton(container: HTMLElement): HTMLButtonElement {
+  const button = Array.from(container.querySelectorAll('button')).find(
+    (b) => b.textContent?.trim() === 'Cut',
+  );
+  assert.ok(button, 'expected the panel to render its Cut button');
+  return button as HTMLButtonElement;
+}
+
+/** Stub `splitWallAtDistance` with a successful split reporting `openings`. */
+function seedSplit(openings: { toLeft: number; toRight: number; skipped: number }) {
+  useViewerStore.setState({
+    splitWallAtDistance: () => ({
+      ok: true,
+      left: { expressId: 1, globalId: 101 },
+      right: { expressId: 2, globalId: 102 },
+      openings,
+    }),
+  } as unknown as Partial<ReturnType<typeof useViewerStore.getState>>);
+}
+
+describe('SplitNumericInput: wall-split notices', () => {
+  const original = {
+    splitWallAtDistance: useViewerStore.getState().splitWallAtDistance,
+    info: toast.info,
+    success: 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({
+      activeTool: 'split',
+      splitMode: 'aiming',
+      splitHoverPoint: [0, 0, 0],
+      splitHoverDistance: 1.5,
+      splitHoverLength: 3,
+      splitTargetModelId: 'm1',
+      splitTargetExpressId: 42,
+      cameraCallbacks: { projectToScreen: () => ({ x: 10, y: 10 }) },
+      clearSplitHover: () => {},
+      setSelectedEntityId: () => {},
+    } as unknown as Partial<ReturnType<typeof useViewerStore.getState>>);
+  });
+
+  afterEach(() => {
+    cleanup();
+    (toast as { info: (m: string) => void }).info = original.info;
+    (toast as { success: (m: string) => void }).success = original.success;
+    useViewerStore.setState({ splitWallAtDistance: original.splitWallAtDistance });
+  });
+
+  it('warns about openings the split could not reassign', () => {
+    seedSplit({ toLeft: 1, toRight: 0, skipped: 2 });
+
+    const container = render(<SplitNumericInput />);
+    click(cutButton(container));
+
+    assert.deepEqual(successCalls, ['Wall split (1 opening reassigned) — Ctrl+Z to undo']);
+    assert.ok(
+      infoCalls.some((m) => m.includes('2 openings could not be reassigned')),
+      `expected a skipped-openings notice, got: ${JSON.stringify(infoCalls)}`,
+    );
+  });
+
+  it('stays silent about skipped openings when none were skipped', () => {
+    seedSplit({ toLeft: 1, toRight: 1, skipped: 0 });
+
+    const container = render(<SplitNumericInput />);
+    click(cutButton(container));
+
+    assert.deepEqual(successCalls, ['Wall split (2 openings reassigned) — Ctrl+Z to undo']);
+    assert.deepEqual(infoCalls, []);
+  });
+});

apps/viewer/src/components/viewer/wallSplitNotice.ts

@@ -0,0 +1,79 @@
+/* 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/. */
+
+/**
+ * The user-facing notices a committed wall split emits.
+ *
+ * `MutationSlice.splitWallAtDistance` is reached from TWO places — the canvas
+ * click handler (`selectionHandlers.ts`) and the Split tool's numeric-distance
+ * panel (`tools/SplitNumericInput.tsx`) — and both must report the same split
+ * the same way. They previously each inlined their own copy of the
+ * "(N openings reassigned)" wording, and #3023 taught only the click handler to
+ * also surface `openings.skipped`, so committing the identical split by typing
+ * a distance instead of clicking silently dropped that warning.
+ *
+ * This module is the single definition of both notices. Call
+ * {@link notifyWallSplit} rather than composing the toasts at a call site: a
+ * shared formatter is still one a new call site can forget to call, which is
+ * precisely how the two paths came apart. A shared EMITTER cannot be half-used.
+ *
+ * It deliberately imports nothing but the toast surface, so a call site does
+ * not take on `selectionHandlers.ts`'s store, geometry and measurement imports
+ * just to announce a split.
+ */
+
+import { toast } from '@/components/ui/toast';
+
+/**
+ * `OpeningReassignSummary` as far as the notices care: how many of the source
+ * wall's openings moved to each half, and how many were left on neither.
+ *
+ * Produced by `reassignWallOpenings` (`@/lib/wall-opening-reassign.ts`) and
+ * forwarded verbatim by `splitWallAtDistance`
+ * (`store/slices/mutationSlice.ts:1981`), which also substitutes an all-zero
+ * summary when either half's placement chain does not resolve and the
+ * reassignment is therefore never attempted.
+ */
+export interface OpeningReassignCounts {
+  toLeft: number;
+  toRight: number;
+  skipped: number;
+}
+
+/**
+ * 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: "0 openings
+ * moved" is noise for a wall with no doors or windows.
+ */
+export function formatOpeningReassignSuffix(op: OpeningReassignCounts): string {
+  const moved = op.toLeft + op.toRight;
+  return moved > 0 ? ` (${moved} opening${moved === 1 ? '' : 's'} reassigned)` : '';
+}
+
+/**
+ * Announce a wall split that has already been committed.
+ *
+ * Always emits the success toast. Additionally warns when
+ * `openings.skipped > 0`: those openings stay attached to the source wall the
+ * split has just tombstoned rather than moving to either half, so they can end
+ * up orphaned.
+ *
+ * `skipped` is incremented at twelve distinct sites in
+ * `@/lib/wall-opening-reassign.ts` (lines 123-203), only one of which is an
+ * unresolvable placement chain. The rest are an attribute the reader could not
+ * read or a reference the opening does not carry — and one is not a fault at
+ * all: an opening whose `PlacementRelTo` points somewhere other than the source
+ * wall is skipped on purpose, because rewriting its parent placement would
+ * teleport it. Zero is the ordinary outcome, which is why this stays quiet
+ * then; it must not be quiet on ONE of the two commit paths when it is not.
+ */
+export function notifyWallSplit(op: OpeningReassignCounts): void {
+  toast.success(`Wall split${formatOpeningReassignSuffix(op)} — Ctrl+Z to undo`);
+  if (op.skipped > 0) {
+    toast.info(
+      `${op.skipped} opening${op.skipped === 1 ? '' : 's'} could not be reassigned and may need manual repositioning`,
+    );
+  }
+}