fix(editor): zone drafting respects the snapping mode + shows its HUD chip
Two zone fixes. helper-manager: the contextual HUD only rendered for tools with `def.toolHints`, so zone (none) showed no HUD and no snapping chip even though it resolves a snap context. Render the generic RegisteredToolHelper whenever the tool has hints OR a snap/continuation context; hoist the legacy `roof` RoofHelper above it so the new fallback doesn't capture it. Any snappable hint-less draft tool now advertises Shift = cycle. zone-tool: a not-yet-migrated legacy tool — it used Shift as a snap bypass and applied `gridSnapStep` unconditionally, so Off mode still snapped to grid. Migrated to the mode-driven exclusive-modes convention (zone resolves to the 'wall' context): grid quantize gated on isGridSnapActive(), 15° ray gated on isAngleSnapActive(), Off/Lines leave the raw cursor. Dropped the Shift-bypass and its key listeners — Shift now cycles the mode globally. Recorded as migrated in the review skill's known-legacy list. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Wassim SAMAD
co-authored by
Claude Opus 4.8
Claude Opus 4.8
parent
a2f1ef1683
commit
15d36c12b1
@@ -148,7 +148,7 @@ Apply when the diff touches a tool, a `move-tool` / `selection` / endpoint / res
|
||||
- **Snappable kinds declare `snapProfile`.** A kind whose tool snaps but whose `NodeDefinition` omits `snapProfile` (`'item' | 'structural'`) gets no contextual chip and the wrong default mode-set — flag it (suggestion, blocker if it ships a bespoke per-kind snapping switch instead).
|
||||
- **Bespoke movers must not open a `moving` scope.** `useMovingNode()` reads the scope, and `tool-manager` mounts the generic `MoveRegistryNodeTool` whenever it's non-null. A bespoke `move-tool.tsx` that calls `begin(movingScope(...))` or `setMovingNode(node)` re-creates the dual-path double-handling (FPS collapse / teleport on move). **Blocker.** Mode-driven snapping inside a bespoke mover must resolve the mode without a global `moving` / `reshaping` scope (see `interaction-scope.md` § "Snapping mode & modifiers").
|
||||
- **`event.altKey` is not an alignment bypass.** A drafting/preview path that reads `event.altKey` to suppress Figma-alignment is a **blocker** in any **new** or **touched** tool — alignment follows the magnetic snap mode (`bypass: !isMagneticSnapActive()`). Alt is force/free for placement/move; it is **not** a snap/alignment modifier. The one sanctioned Alt use outside force is the **wall/fence chain-mode toggle** (clean Alt-tap → `cycleWallChainMode` / `cycleFenceChainMode`, via `hooks/use-keyboard.ts` `isChainModeContext()`), allowed only because wall/fence drafting has no force role. Grep tell: `event.altKey` near an `align` / `bypass` expression in a `tool.tsx` / floorplan preview path.
|
||||
- **Known-legacy exceptions (migrate on touch).** Tracked debt in `plans/editor-placement-interaction-overhaul.md`; a PR that **touches** one must migrate it, not extend it; a **new** tool on either legacy pattern is a blocker regardless. (1) `shiftKey` snap-bypass in the MEP move/endpoint tools (`packages/nodes/src/{duct-segment,pipe-segment,liquid-line,lineset,duct-fitting}/{move-tool,selection}.tsx`). (2) `altKey` alignment-bypass in the roof / polygon / slab pointer-move previews (`components/editor/floorplan-panel.tsx`) and the `resolveSlabPlanPointSnap` / `resolveCeilingPlanPointSnap` paths. **Already migrated — do not regress:** wall + fence drafting (both modifier patterns).
|
||||
- **Known-legacy exceptions (migrate on touch).** Tracked debt in `plans/editor-placement-interaction-overhaul.md`; a PR that **touches** one must migrate it, not extend it; a **new** tool on either legacy pattern is a blocker regardless. (1) `shiftKey` snap-bypass in the MEP move/endpoint tools (`packages/nodes/src/{duct-segment,pipe-segment,liquid-line,lineset,duct-fitting}/{move-tool,selection}.tsx`). (2) `altKey` alignment-bypass in the roof / polygon / slab pointer-move previews (`components/editor/floorplan-panel.tsx`) and the `resolveSlabPlanPointSnap` / `resolveCeilingPlanPointSnap` paths. **Already migrated — do not regress:** wall + fence drafting (both modifier patterns) and `zone` drafting (`components/tools/zone/zone-tool.tsx` — mode-driven grid/angle gates, no Shift bypass).
|
||||
|
||||
## 5. Output format
|
||||
|
||||
|
||||
@@ -13,7 +13,7 @@ import { BufferGeometry, DoubleSide, type Group, type Line, Shape, Vector3 } fro
|
||||
import { EDITOR_LAYER } from './../../../lib/constants'
|
||||
import { sfxEmitter } from './../../../lib/sfx-bus'
|
||||
import { snapWorldXZForActiveBuilding } from './../../../lib/world-grid-snap'
|
||||
import useEditor from './../../../store/use-editor'
|
||||
import useEditor, { isAngleSnapActive, isGridSnapActive } from './../../../store/use-editor'
|
||||
import { CursorSphere } from '../shared/cursor-sphere'
|
||||
|
||||
const Y_OFFSET = 0.02
|
||||
@@ -65,7 +65,6 @@ export const ZoneTool: React.FC = () => {
|
||||
const pointsRef = useRef<Array<[number, number]>>([])
|
||||
const previousSnappedPointRef = useRef<[number, number] | null>(null)
|
||||
const levelYRef = useRef(0) // Track current level Y position
|
||||
const shiftPressed = useRef(false)
|
||||
const currentLevelId = useViewer((state) => state.selection.levelId)
|
||||
const setTool = useEditor((state) => state.setTool)
|
||||
|
||||
@@ -86,21 +85,19 @@ export const ZoneTool: React.FC = () => {
|
||||
mainLineRef.current.geometry = new BufferGeometry()
|
||||
closingLineRef.current.geometry = new BufferGeometry()
|
||||
|
||||
// 15° angle snap from the last vertex by default. Shift bypasses all snap.
|
||||
// Distance snaps along the ray so the vertex lands on
|
||||
// grid-multiple lengths without leaving the ray.
|
||||
// Snapping follows the active mode (zone resolves to the 'wall' context):
|
||||
// `angles` locks the ray to 15° from the last vertex, `grid` quantizes the
|
||||
// distance along it, `lines` / `off` leave the raw cursor. No held-Shift
|
||||
// bypass — Shift cycles the mode (see interaction-scope.md).
|
||||
const snapDraftPoint = (
|
||||
lastPoint: [number, number],
|
||||
gridPoint: [number, number],
|
||||
_gridPoint: [number, number],
|
||||
rawPoint: [number, number],
|
||||
): [number, number] => {
|
||||
if (shiftPressed.current) return rawPoint
|
||||
const [x, z] = snapPointAlongAngleRay(
|
||||
lastPoint,
|
||||
rawPoint,
|
||||
DEFAULT_ANGLE_STEP,
|
||||
useEditor.getState().gridSnapStep,
|
||||
)
|
||||
const angleStep = isAngleSnapActive() ? DEFAULT_ANGLE_STEP : 0
|
||||
const gridStep = isGridSnapActive() ? useEditor.getState().gridSnapStep : 0
|
||||
if (angleStep === 0 && gridStep === 0) return rawPoint
|
||||
const [x, z] = snapPointAlongAngleRay(lastPoint, rawPoint, angleStep, gridStep)
|
||||
return [x, z]
|
||||
}
|
||||
|
||||
@@ -172,15 +169,16 @@ export const ZoneTool: React.FC = () => {
|
||||
if (!cursorRef.current) return
|
||||
|
||||
// World-grid snap projected into building-local; rotated buildings
|
||||
// used to pull the snap off the visible grid lines.
|
||||
const bypassSnap = shiftPressed.current || event.nativeEvent?.shiftKey === true
|
||||
const [gridX, gridZ] = bypassSnap
|
||||
? [event.localPosition[0], event.localPosition[2]]
|
||||
: snapWorldXZForActiveBuilding(
|
||||
// used to pull the snap off the visible grid lines. Grid quantize only
|
||||
// in grid mode; off / lines / angles leave the raw cursor for the first
|
||||
// vertex (later vertices snap along the ray in `snapDraftPoint`).
|
||||
const [gridX, gridZ] = isGridSnapActive()
|
||||
? snapWorldXZForActiveBuilding(
|
||||
event.position[0],
|
||||
event.position[2],
|
||||
useEditor.getState().gridSnapStep,
|
||||
).local
|
||||
: [event.localPosition[0], event.localPosition[2]]
|
||||
cursorPosition = [gridX, gridZ]
|
||||
rawCursorPosition = [event.localPosition[0], event.localPosition[2]]
|
||||
levelYRef.current = event.localPosition[1]
|
||||
@@ -191,9 +189,10 @@ export const ZoneTool: React.FC = () => {
|
||||
? snapDraftPoint(lastPoint, cursorPosition, rawCursorPosition)
|
||||
: cursorPosition
|
||||
|
||||
// Play snap sound when the snapped position changes during drawing
|
||||
// Play snap sound when the snapped position changes during drawing — only
|
||||
// when a quantizing mode is active (off / lines move continuously).
|
||||
if (
|
||||
!bypassSnap &&
|
||||
(isGridSnapActive() || isAngleSnapActive()) &&
|
||||
pointsRef.current.length > 0 &&
|
||||
previousSnappedPointRef.current &&
|
||||
(displayPoint[0] !== previousSnappedPointRef.current[0] ||
|
||||
@@ -211,14 +210,13 @@ export const ZoneTool: React.FC = () => {
|
||||
const onGridClick = (event: GridEvent) => {
|
||||
if (!currentLevelId) return
|
||||
|
||||
const bypassSnap = shiftPressed.current || event.nativeEvent?.shiftKey === true
|
||||
const [gridX, gridZ] = bypassSnap
|
||||
? [event.localPosition[0], event.localPosition[2]]
|
||||
: snapWorldXZForActiveBuilding(
|
||||
const [gridX, gridZ] = isGridSnapActive()
|
||||
? snapWorldXZForActiveBuilding(
|
||||
event.position[0],
|
||||
event.position[2],
|
||||
useEditor.getState().gridSnapStep,
|
||||
).local
|
||||
: [event.localPosition[0], event.localPosition[2]]
|
||||
let clickPoint: [number, number] = [gridX, gridZ]
|
||||
|
||||
// Snap to the 15° ray from the last point
|
||||
@@ -270,28 +268,12 @@ export const ZoneTool: React.FC = () => {
|
||||
}
|
||||
}
|
||||
|
||||
const onKeyDown = (e: KeyboardEvent) => {
|
||||
if (e.key === 'Shift') shiftPressed.current = true
|
||||
}
|
||||
const onKeyUp = (e: KeyboardEvent) => {
|
||||
if (e.key === 'Shift') shiftPressed.current = false
|
||||
}
|
||||
const onWindowBlur = () => {
|
||||
shiftPressed.current = false
|
||||
}
|
||||
document.addEventListener('keydown', onKeyDown)
|
||||
document.addEventListener('keyup', onKeyUp)
|
||||
window.addEventListener('blur', onWindowBlur)
|
||||
|
||||
// Subscribe to events
|
||||
emitter.on('grid:move', onGridMove)
|
||||
emitter.on('grid:click', onGridClick)
|
||||
emitter.on('grid:double-click', onGridDoubleClick)
|
||||
|
||||
return () => {
|
||||
document.removeEventListener('keydown', onKeyDown)
|
||||
document.removeEventListener('keyup', onKeyUp)
|
||||
window.removeEventListener('blur', onWindowBlur)
|
||||
emitter.off('grid:move', onGridMove)
|
||||
emitter.off('grid:click', onGridClick)
|
||||
emitter.off('grid:double-click', onGridDoubleClick)
|
||||
|
||||
@@ -181,16 +181,25 @@ export function HelperManager() {
|
||||
return <ContextualHelperPanel hints={selectModeHints} />
|
||||
}
|
||||
|
||||
// Registry-first: kinds with `def.toolHints` render through the generic
|
||||
// `RegisteredToolHelper`. Today that covers ceiling / door / fence /
|
||||
// item / shelf / slab / spawn / wall / window.
|
||||
// Legacy fallback — only `roof` remains because it hasn't migrated to
|
||||
// `def.tool` / `def.toolHints` yet (no Stage D port). Checked before the
|
||||
// generic tool branch so the snap-context fallback below doesn't capture it
|
||||
// and drop its bespoke `RoofHelper` hints. When roof migrates, this deletes.
|
||||
if (tool === 'roof') return <RoofHelper snapContext={snapContext} />
|
||||
|
||||
// Registry-first: a kind renders the generic `RegisteredToolHelper` when it
|
||||
// declares `def.toolHints`, OR whenever its draft resolves to a snap /
|
||||
// continuation context — so a snappable tool with NO hand-written hints (e.g.
|
||||
// `zone`) still advertises the snapping chip it already honors (Shift = cycle).
|
||||
// `RegisteredToolHelper` self-hides when there's genuinely nothing to show.
|
||||
if (tool) {
|
||||
const def = nodeRegistry.get(tool)
|
||||
if (def?.toolHints && def.toolHints.length > 0) {
|
||||
const hints = def?.toolHints ?? []
|
||||
if (hints.length > 0 || snapContext || continuationContext) {
|
||||
return (
|
||||
<RegisteredToolHelper
|
||||
continuationContext={continuationContext}
|
||||
hints={def.toolHints}
|
||||
hints={hints}
|
||||
shiftPressed={modifiers.shift}
|
||||
snapContext={snapContext}
|
||||
/>
|
||||
@@ -198,9 +207,5 @@ export function HelperManager() {
|
||||
}
|
||||
}
|
||||
|
||||
// Legacy fallback — only `roof` remains because it hasn't migrated to
|
||||
// `def.tool` / `def.toolHints` yet (no Stage D port). When roof
|
||||
// migrates, this switch deletes outright.
|
||||
if (tool === 'roof') return <RoofHelper snapContext={snapContext} />
|
||||
return null
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user