fix: SSR-safe portals, deduplicate image validation, z-index custom properties, and misc hardening

[bensonwong]

Summary

Addresses leftover PR feedback from the CitationDrawer redesign (#196). This PR hardens the component layer with SSR safety, deduplication, customizability, and code quality improvements across 7 files.

Changes

• SSR-safe portal rendering: Added getPortalContainer() utility that returns document.body or null during SSR. All three createPortal call sites (CitationDrawer, CitationComponent ImageOverlay, SourcesListComponent) now guard against missing document.body instead of crashing during server-side rendering.

• Deduplicated image validation: Extracted isValidProofImageSrc(), SAFE_DATA_IMAGE_PREFIXES, and TRUSTED_IMAGE_HOSTS from CitationDrawer.tsx into constants.ts. Replaced the ~20-line inline duplicate in CitationDrawerTrigger.tsx with a single function call. Both components now share the same validation logic from one canonical location.

• Z-index CSS custom properties: Replaced hardcoded Tailwind z-[9998]/z-[9999] classes with CSS custom properties (--dc-z-popover, --dc-z-drawer-backdrop, --dc-z-drawer, --dc-z-image-overlay). Consumers can now override stacking order via CSS without forking components. Added a layering hierarchy table in constants.ts documenting the z-index architecture.

• Dev-mode deprecation warnings: Added a module-level Set-based "warn once" pattern to CitationComponent so deprecated prop warnings (interactionMode, eventHandlers.onClick + behaviorConfig.onClick conflict) only fire once per prop instead of on every render.

• Tooltip max-height constraint: Added max-h-[50vh] overflow-y-auto to SourceTooltip in CitationDrawerTrigger to prevent unbounded growth with long source names or many citations.

• Magic numbers extracted: Moved SPINNER_TIMEOUT_MS and HOVER_CLOSE_DELAY_MS to module-level named constants in CitationComponent.tsx. Added ICON_EXPANDED_MARGIN_PX and ICON_COLLAPSED_MARGIN_PX in CitationDrawerTrigger.tsx replacing inline 6 / -8 values.

• requestAnimationFrame for focus timing: ImageOverlay auto-focus now uses requestAnimationFrame with proper cleanup via cancelAnimationFrame, ensuring the portal has been painted before focusing.

Test plan

• All 1028 existing tests pass (npx bun test src/__tests__/)
• Build succeeds (npm run build)
• Verify SSR: components should return null instead of crashing when document is undefined
• Verify z-index overrides: setting --dc-z-drawer: 50000 in CSS should raise the drawer above other overlays
• Verify deprecation warnings fire once in dev console, not on every render

[github-actions]

✅ Playwright Test Report

Status: Tests passed

📊 Download Report & Snapshots (see Artifacts section)

What's in the Visual Snapshots

The gallery includes visual snapshots for:

• 🖥️ Desktop showcase (all variants × all states)
• 📱 Mobile showcase (iPhone SE viewport)
• 📟 Tablet showcase (iPad viewport)
• 🔍 Popover states (verified, partial, not found)
• 🔗 URL citation variants

---

Run ID: 21896542065

[claude]

Code Review: SSR-safe portals, deduplicate image validation, z-index custom properties

Overall, this is a high-quality hardening PR that addresses multiple technical debt items with clean, well-documented code. The changes improve SSR compatibility, code reusability, customizability, and developer experience.

---

✅ Strengths

1. SSR Safety - Well Implemented

The getPortalContainer() utility is a clean, reusable solution that correctly checks for document availability and is applied consistently across all portal call sites (CitationDrawer, ImageOverlay, SourcesListComponent). Components properly return null when container unavailable.

2. Code Deduplication - Excellent

Extracting isValidProofImageSrc() eliminates ~20 lines of duplicate logic with a single canonical location in constants.ts. Security-focused validation blocks SVG data URIs and validates HTTPS/trusted hosts, with proper type guard return signature.

3. Z-Index Architecture - Well Designed

The CSS custom property approach is flexible and well-documented with clear hierarchy table in comments. Allows consumer customization without forking components, with consistent application and fallback values.

4. Developer Experience Improvements

• "Warn once" pattern prevents console spam using Set-based tracking
• Magic numbers extracted to named constants (SPINNER_TIMEOUT_MS, HOVER_CLOSE_DELAY_MS)
• Improved focus timing with requestAnimationFrame + proper cleanup

5. Code Quality

• Proper cleanup in useEffect hooks (cancelAnimationFrame)
• Max-height constraint on tooltip prevents unbounded growth
• Follows CLAUDE.md "no re-export variables" rule (exports from canonical locations)

---

💡 Suggestions & Observations

1. Type Safety in Z-Index Styles

The type assertion for CSS custom properties in zIndex styles is necessary because TypeScript's CSSProperties type doesn't fully support CSS custom property strings. Consider documenting this in a comment for future maintainers.

2. requestAnimationFrame Cleanup Timing

The cleanup properly cancels pending animation frames if the component unmounts before the next paint - this is correct implementation.

3. Image Validation Security

The isValidProofImageSrc() function correctly blocks SVG data URIs which are an XSS vector. Good security practice. Consider documenting why SVG is excluded in a comment for future maintainers.

4. Tooltip Overflow Handling

The max-h-[50vh] overflow-y-auto approach may cause slight layout shifts when scrollbar appears. Consider using overflow-y-scroll to reserve scrollbar space (minor UX consideration, not a blocker).

5. Deprecation Warning Conflicts

The warning for eventHandlers.onClick being ignored when behaviorConfig.onClick is provided is good DX. Consider if you need similar handling in the opposite scenario.

---

🔍 Potential Issues

Minor: Z-Index Default Values Could Conflict

Both popover and drawer backdrop use 9998. This is likely intentional (both are backdrop layers), but if a popover is open inside a drawer, they'll be at the same level. Document if this is the intended behavior.

Minor: SSR Portal Check Placement

The SSR check happens after all hooks run. This is fine in practice (hooks won't cause issues), but React officially discourages conditional returns after hooks. Consider moving the check earlier if possible, or document that hooks are SSR-safe.

---

🧪 Testing Recommendations

The PR mentions manual testing is needed. Suggest adding:

1. Unit tests for new utilities:

   • getPortalContainer: test browser and SSR scenarios
   • isValidProofImageSrc: test safe data URIs, SVG rejection, HTTPS validation, HTTP/untrusted host rejection

2. SSR rendering test:
   Mock document as undefined and verify components don't crash and return empty DOM

---

📋 Checklist

• ✅ Code quality: Clean, well-organized, follows project conventions
• ✅ Best practices: Proper hook cleanup, SSR safety, security validation
• ✅ Performance: No regressions, requestAnimationFrame properly used
• ✅ Security: SVG data URI blocking, HTTPS validation
• ⚠️ Test coverage: Manual testing planned, automated tests recommended
• ✅ Documentation: Excellent inline comments and hierarchy tables
• ✅ No variable re-exports (per CLAUDE.md rules)

---

🎯 Recommendation

Approve with suggestions. This PR is production-ready. The suggested improvements are minor enhancements, not blockers.

Priority: Consider adding unit tests for getPortalContainer() and isValidProofImageSrc() to prevent regressions.

---

Great work on this hardening pass! The codebase is significantly more robust after these changes. 🚀