feat: migrate Dialog component from @headlessui/react to @radix-ui/react-dialog

[ErnieAtLYD]

• Replace the Dialog component implementation from using @headlessui/react to @radix-ui/react-dialog for enhanced accessibility features. Update usage examples and documentation to reflect the new API and add backward compatibility notes for the legacy Dialog API.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
reflect-app	Ready	Preview	Comment	Aug 31, 2025 3:27pm

[openhands-ai]

Looks like there are a few issues preventing this PR from being merged!

• GitHub Actions are failing:
  • Claude Code Review
  • CI/CD Pipeline

If you'd like me to help, just leave a comment, like

@OpenHands please fix the failing actions on PR #38 at branch `35-fix-headlessuijsdom-compatibility-issues-in-component-tests`

Feel free to include any additional details that might help me get this PR into a better state.

_{^{You can manage your notification settings}}

[claude]

Claude finished @ErnieAtLYD's task —— View job

---

Dialog Component Migration Review

I've completed my analysis of the PR that migrates the Dialog component from @headlessui/react to @radix-ui/react-dialog. Here's my comprehensive review:

✅ Code Quality & Best Practices

Excellent implementation with several highlights:

1. Dual API Design: Smart approach providing both legacy and new composition APIs for backward compatibility
2. Type Safety: Full TypeScript support with proper variant props and forwarded refs
3. Accessibility: Leverages Radix UI's excellent accessibility features (focus management, ARIA attributes, keyboard handling)
4. CSS Architecture: Clean use of class-variance-authority for size variants with proper Tailwind integration

Minor Suggestions:

• Consider adding JSDoc comments for the new composition components (DialogRoot, DialogContent, etc.) to improve developer experience
• The panelProps type casting in tests (line 118) could be cleaner with a helper type

✅ Migration Strategy

Well-executed migration:

• Legacy Dialog component maintains exact same API - no breaking changes
• New Radix composition API available for modern usage patterns
• Proper dependency updates: @radix-ui/react-dialog@^1.1.15 replaces @headlessui/react
• Smart test adaptation focusing on behavior rather than implementation details

⚠️ Test Coverage Concerns

Issues identified:

1. Weak Integration Testing: Tests at lines 68-95 only verify that onClose prop is defined, not that it's actually called when users interact:

   // Current - only checks prop exists
   expect(onClose).toBeDefined()
   
   // Should test actual behavior
   expect(onClose).toHaveBeenCalled()

2. Missing Focus Management Tests: The focus management test file references Dialog components but actual focus trap/restoration testing is commented out or simplified

3. HeadlessUI Behavior Gap: Comments reference e2e tests for "comprehensive HeadlessUI behavior testing" but this creates a gap in unit test coverage for critical dialog behaviors

Recommendations:

• Add integration tests that actually trigger Radix UI's event handlers
• Test ESC key behavior by simulating the event on the dialog element
• Test backdrop click behavior (if supported by Radix)
• Verify focus trapping works correctly

✅ Performance Considerations

Good performance characteristics:

• Radix UI is generally more performant than HeadlessUI
• Proper use of React.forwardRef and React.useCallback
• Efficient animation classes using Tailwind's built-in animations
• Portal rendering handled by Radix UI optimally

✅ Security Assessment

No security concerns identified:

• No XSS vulnerabilities in the implementation
• Proper prop spreading with TypeScript safety
• No dangerous DOM manipulation
• Live regions integration appears safe

❌ Potential Issues

1. Runtime Dependency: Ensure @/lib/live-regions is properly implemented and tested
2. Animation Conflicts: The complex animation class string in DialogContent (line 57) could conflict - consider extracting to a variant
3. Test ID Duplication: Both legacy and new APIs use data-testid="dialog-panel" which could cause conflicts if both are used simultaneously

🔧 Recommended Actions

1. High Priority: Strengthen test coverage for user interactions and focus management
2. Medium Priority: Add JSDoc documentation for new composition API
3. Low Priority: Consider extracting complex animation classes to variants

📊 Overall Assessment

This is a high-quality migration that successfully modernizes the Dialog component while maintaining backward compatibility. The implementation follows React and accessibility best practices. The main concern is insufficient test coverage for user interactions, but the code architecture is solid.

Recommendation: Approve after addressing test coverage gaps.

---

Files Analyzed:

• src/components/ui/dialog.tsx - Main implementation ✅
• src/components/__tests__/dialog.test.tsx - Test coverage ⚠️
• package.json - Dependency changes ✅
• src/test/__tests__/focus-management.test.tsx - Integration tests ✅

---

[claude]

Claude finished @ErnieAtLYD's task —— View job

---

Dialog Component Migration Review

Reviewing the migration from @headlessui/react to @radix-ui/react-dialog...

Todo List

• Analyze main Dialog component implementation
• Review test coverage and quality
• Check package.json dependency changes
• Evaluate migration strategy and backward compatibility
• Assess code quality and best practices
• Check for potential bugs and security issues
• Evaluate performance implications
• Provide comprehensive feedback

---

✅ Code Quality & Best Practices

Excellent implementation with strong points:

1. Dual API Design - Smart strategy providing both legacy Dialog component (src/components/ui/dialog.tsx:262-322) and new Radix composition API (DialogRoot, DialogContent, etc.) for seamless backward compatibility
2. Type Safety - Comprehensive TypeScript support with proper VariantProps, forwardRef usage, and well-defined interfaces
3. Documentation - Good JSDoc comments for all exported components with usage examples
4. Accessibility Excellence - Leverages Radix UI's superior accessibility features including automatic ARIA attributes, focus management, and keyboard handling
5. CSS Architecture - Clean implementation using class-variance-authority for size variants with Tailwind integration

Minor Suggestions:

• Consider extracting the complex animation class string in DialogContent (line 106) to a separate variant for better maintainability
• The panelProps type assertion in tests (dialog.test.tsx:133) could be cleaner with a helper type

✅ Migration Strategy

Well-executed approach:

• Zero Breaking Changes - Legacy Dialog component maintains identical API
• Progressive Enhancement - New composition API available for modern usage patterns
• Proper Dependencies - Clean swap from @headlessui/react to @radix-ui/react-dialog@^1.1.15
• Documentation Updated - CLAUDE.md shows both APIs with clear usage examples

⚠️ Test Coverage Concerns

Issues identified:

1. Insufficient User Interaction Testing - Tests at dialog.test.tsx:69-110 only verify that onClose prop exists rather than testing actual user interactions:

   // Current approach only checks prop definition
   expect(onClose).toBeDefined()
   
   // Missing: Actual ESC key behavior testing
   // Missing: Backdrop click interaction testing

2. Focus Management Gap - The focus-management test file includes Dialog components but actual focus trap testing is minimal for the Dialog specifically

3. Radix-Specific Behavior - Tests rely on e2e for "comprehensive HeadlessUI behavior testing" but this creates coverage gaps for unit testing of critical dialog behaviors

Recommendations:

• Add integration tests that actually trigger Radix UI's event handlers
• Test ESC key behavior by simulating KeyboardEvent on document/dialog
• Test actual user interactions rather than just prop definitions
• Verify focus trapping works in unit tests

✅ Performance & Security

Performance improvements:

• Radix UI is generally more performant than HeadlessUI
• Proper React patterns with useCallback and forwardRef
• Efficient portal rendering and animation handling

Security assessment:

• No security vulnerabilities identified
• Proper prop spreading with TypeScript safety
• Safe integration with @/lib/live-regions for screen reader announcements

❌ Potential Issues

1. Test ID Collision - Both legacy and new APIs use data-testid="dialog-panel" which could cause conflicts if used simultaneously
2. Animation Complexity - The long animation class string could be fragile and hard to debug
3. Runtime Dependency - Ensure @/lib/live-regions implementation is robust and tested

📊 Overall Assessment

This is a high-quality migration that successfully modernizes the Dialog component while maintaining complete backward compatibility. The implementation demonstrates excellent React and accessibility practices.

Key Strengths:

• Zero breaking changes for existing users
• Superior accessibility with Radix UI
• Clean TypeScript implementation
• Comprehensive documentation

Main Concern:
Insufficient test coverage for user interactions - tests verify component structure but not actual user behavior (ESC key, backdrop clicks, focus management).

Recommendation: ✅ Approve after addressing test coverage gaps

The code quality is excellent and the migration strategy is sound. The primary improvement needed is more robust unit testing of user interactions to ensure the Radix UI integration works as expected.

---

Files Analyzed:

• src/components/ui/dialog.tsx - Main implementation ✅
• src/components/__tests__/dialog.test.tsx - Test coverage ⚠️
• package.json - Dependency changes ✅
• CLAUDE.md - Documentation updates ✅

---