# 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:
```tsx
// 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: ✅