HartOMat/review-report.md

Review Report: CAD Viewer Material Assignment Fix + Feature Parity

Datum: 2026-03-10

Ergebnis: ✅ Freigabe

---

Gefundene Probleme

[InlineCadViewer.tsx + ThreeDViewer.tsx] Misleading comment on isolateMode reset effect

Schwere: Gering (Kommentar)

In both files the comment reads:

// Reset isolateMode and hideAssigned when no part is pinned
useEffect(() => {
  if (!pinnedPart) setIsolateMode('none')   // ← only resets isolateMode, not hideAssigned!
}, [pinnedPart])

The comment says "and hideAssigned" but the effect only calls setIsolateMode('none'). The behavior is actually correct — hideAssigned should NOT be reset when unpinning (it's a persistent view toggle). Only the comment is wrong.

Empfehlung: Change to // Reset isolateMode when no part is pinned.

---

Positiv aufgefallen

Bug fix: MaterialPanel invisible in ThreeDViewer — root cause correctly identified

The diagnosis was precise: the outer <div onClick={() => setPinnedPart(null)}> was receiving the native DOM bubble from every canvas click, calling setPinnedPart(null) in the same React batch as setPinnedPart(name) from the THREE.js event handler — final state always null.

The two-part fix is clean and idiomatic:

• onClick={(e) => e.stopPropagation()} on the viewport div absorbs DOM clicks
• onPointerMissed={() => setPinnedPart(null)} on the R3F Canvas handles the "click empty space" case via the THREE.js raycaster (fires only when no mesh is hit) — this is exactly the right R3F API for this use case

cadUtils.ts — normalization regex extension

/_AF\d+(_ASM)?$/i is minimal and correct. It handles:

• _AF0, _AF1 (existing, unchanged)
• _AF0_ASM, _AF1_ASM (new — assembly-node suffix)
• Case-insensitive flag is defensive and correct
• The loop-until-stable pattern handles nested suffixes as before
• _ASM alone (without _AF\d+) is NOT stripped — correct, it's part of base names like GE360-HF_000_P_ASM_ASM

Combined visibility useEffect — correct design

Merging hideAssigned + isolateMode into a single traversal effect avoids ordering ambiguity between two independent effects competing on the same mesh.visible and mat.opacity. The priority order (hideAssigned first, then isolateMode) is explicit and logical. The pinned part (isSelected) is always protected from hiding regardless of mode. ✓

Effect separation is clean

• Color-apply effect: only touches mat.color → deps [modelReady, partMaterials]
• Unassigned glow effect: only touches mat.emissive → deps [modelReady, showUnassigned, partMaterials]
• Combined visibility effect: only touches mesh.visible / mat.opacity / mat.transparent → deps [modelReady, pinnedPart, isolateMode, hideAssigned, partMaterials]

No effect touches another effect's properties — no race conditions.

GPU hint and DPR cap

gl={{ powerPreference: 'high-performance' }} + dpr={[1, 1.5]} on both Canvas elements. preserveDrawingBuffer: true correctly kept only in ThreeDViewer (required for screenshot capture).

"Hide assigned" toolbar button correctly conditional

{assignedCount > 0 && (...)} in InlineCadViewer and {modelReady && Object.keys(partMaterials).length > 0 && (...)} in ThreeDViewer — button only appears when there is something to hide.

Debug log is dev-only

if (!import.meta.env.DEV || ...) guard ensures the console output and traversal overhead never reach production. The output logs both matched and unmatched keys, which is exactly what's needed to diagnose remaining name mismatches after the normalization fix.

Feature parity achieved

ThreeDViewer and InlineCadViewer now have matching material-assignment features:

• ✓ showUnassigned highlight toggle with count badge
• ✓ hideAssigned toggle (new, both viewers)
• ✓ isolateMode (ghost / hide) via MaterialPanel (both viewers)
• ✓ onPointerMissed closes panel on empty-space click in ThreeDViewer

---

Empfehlung

Freigabe. The one Gering comment issue can be fixed inline.

Review abgeschlossen. Ergebnis: ✅