Files
leptos-shadcn-ui/docs/remediation/components/dialog/REMEDIATION_PLAN.md
Peter Hanssens ec459d142c 🔧 Complete refactoring implementation for v0.9.0
- All 9 components fully refactored with modular architecture
- 45+ test modules created and organized
- File size compliance achieved (99% reduction)
- Enterprise-grade code quality implemented
- All compilation issues resolved

Ready for v0.9.0 release publication!
2025-09-22 13:06:22 +10:00

17 KiB

🔧 Dialog Component Remediation Plan

Priority 1: Critical Modal Component - Immediate Action Required

🚨 Critical Issues Summary

The Dialog component has severe issues that make it unsuitable for production use:

  • ⚠️ 2 test files with basic implementations only
  • ⚠️ ~20% test coverage (estimated)
  • Missing accessibility features (ARIA attributes, focus management)
  • No keyboard navigation (Escape key, Tab management)
  • Missing modal behavior tests
  • No focus trap functionality

🎯 Remediation Strategy

Phase 1: Critical Accessibility Features (Week 1)

Day 1-2: Add ARIA Attributes

Current Problem: Dialog lacks proper ARIA attributes for accessibility

Target Implementation:

// Enhanced DialogContent with ARIA attributes
#[component]
pub fn DialogContent(
    #[prop(into, optional)] class: MaybeProp<String>,
    #[prop(optional)] children: Option<Children>,
) -> impl IntoView {
    let ctx = expect_context::<DialogContextValue>();
    
    let content_class = Signal::derive(move || {
        format!("fixed left-1/2 top-1/2 z-50 grid w-full max-w-lg gap-4 border bg-background p-6 shadow-lg duration-200 data-[state=open]:animate-in data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=open]:fade-in-0 data-[state=closed]:zoom-out-95 data-[state=open]:zoom-in-95 data-[state=closed]:slide-out-to-left-1/2 data-[state=closed]:slide-out-to-top-[48%] data-[state=open]:slide-in-from-left-1/2 data-[state=open]:slide-in-from-top-[48%] sm:rounded-lg {}", 
                class.get().unwrap_or_default())
    });

    view! {
        <div
            class={content_class}
            role="dialog"
            aria-modal="true"
            aria-labelledby="dialog-title"
            aria-describedby="dialog-description"
            tabindex="-1"
        >
            {children.map(|c| c())}
        </div>
    }
}

Day 3-4: Implement Focus Management

Current Problem: No focus trap or focus management

Target Implementation:

// Focus management utilities
pub struct FocusManager {
    focusable_elements: Vec<web_sys::Element>,
    first_focusable: Option<web_sys::Element>,
    last_focusable: Option<web_sys::Element>,
}

impl FocusManager {
    pub fn new(container: &web_sys::Element) -> Self {
        let focusable_selector = "button, [href], input, select, textarea, [tabindex]:not([tabindex=\"-1\"])";
        let elements = container.query_selector_all(focusable_selector)
            .unwrap()
            .iter()
            .map(|element| element.dyn_into::<web_sys::Element>().unwrap())
            .collect::<Vec<_>>();
        
        Self {
            first_focusable: elements.first().cloned(),
            last_focusable: elements.last().cloned(),
            focusable_elements: elements,
        }
    }
    
    pub fn trap_focus(&self, event: &web_sys::KeyboardEvent) {
        if event.key() == "Tab" {
            if event.shift_key() {
                // Shift + Tab (backward)
                if document().active_element() == self.first_focusable {
                    event.prevent_default();
                    if let Some(last) = &self.last_focusable {
                        last.focus().unwrap();
                    }
                }
            } else {
                // Tab (forward)
                if document().active_element() == self.last_focusable {
                    event.prevent_default();
                    if let Some(first) = &self.first_focusable {
                        first.focus().unwrap();
                    }
                }
            }
        }
    }
    
    pub fn focus_first(&self) {
        if let Some(first) = &self.first_focusable {
            first.focus().unwrap();
        }
    }
}

Day 5: Implement Keyboard Navigation

Current Problem: No keyboard navigation support

Target Implementation:

// Enhanced Dialog with keyboard navigation
#[component]
pub fn Dialog(
    #[prop(into, optional)] open: Signal<bool>,
    #[prop(into, optional)] on_open_change: Option<Callback<bool>>,
    #[prop(optional)] children: Option<Children>,
) -> impl IntoView {
    let internal_open = RwSignal::new(false);
    let dialog_ref = NodeRef::<html::Div>::new();
    
    let open_state = Signal::derive(move || {
        if open.get() != internal_open.get() {
            open.get()
        } else {
            internal_open.get()
        }
    });

    let set_open = Callback::new(move |new_open: bool| {
        internal_open.set(new_open);
        if let Some(callback) = &on_open_change {
            callback.run(new_open);
        }
    });

    // Handle escape key
    let handle_keydown = Callback::new(move |event: KeyboardEvent| {
        if event.key() == "Escape" {
            set_open.run(false);
        }
    });

    // Focus management effect
    create_effect(move |_| {
        if open_state.get() {
            // Focus first element when dialog opens
            if let Some(element) = dialog_ref.get() {
                let focus_manager = FocusManager::new(&element);
                focus_manager.focus_first();
            }
        }
    });

    provide_context(DialogContextValue {
        open: open_state,
        set_open,
    });

    view! {
        <div
            node_ref=dialog_ref
            on:keydown=handle_keydown
        >
            {children.map(|c| c())}
        </div>
    }
}

Phase 2: Comprehensive Testing (Week 2)

Day 1-2: Modal Behavior Tests

File: packages/leptos/dialog/src/tests/modal_behavior.rs

#[cfg(test)]
mod modal_behavior {
    use super::*;
    use leptos::prelude::*;

    #[test]
    fn test_dialog_opens_and_closes() {
        let (open, set_open) = signal(false);
        
        // Test opening
        set_open.set(true);
        assert!(open.get(), "Dialog should be open");
        
        // Test closing
        set_open.set(false);
        assert!(!open.get(), "Dialog should be closed");
    }

    #[test]
    fn test_dialog_overlay_blocks_interaction() {
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(|| true),
            ..Default::default()
        });
        
        let rendered = dialog.render();
        assert!(rendered.contains("dialog-overlay"));
    }

    #[test]
    fn test_dialog_trigger_opens_dialog() {
        let (open, set_open) = signal(false);
        
        let trigger = DialogTrigger::new(DialogTriggerProps {
            on_click: Some(Callback::new(move |_| {
                set_open.set(true);
            })),
            ..Default::default()
        });
        
        // Simulate trigger click
        trigger.props.on_click.unwrap().run(());
        assert!(open.get(), "Dialog should open when trigger is clicked");
    }

    #[test]
    fn test_dialog_close_button_closes_dialog() {
        let (open, set_open) = signal(true);
        
        let close_button = DialogClose::new(DialogCloseProps {
            on_click: Some(Callback::new(move |_| {
                set_open.set(false);
            })),
            ..Default::default()
        });
        
        // Simulate close button click
        close_button.props.on_click.unwrap().run(());
        assert!(!open.get(), "Dialog should close when close button is clicked");
    }
}

Day 3-4: Accessibility Tests

File: packages/leptos/dialog/src/tests/accessibility.rs

#[cfg(test)]
mod accessibility {
    use super::*;

    #[test]
    fn test_dialog_aria_attributes() {
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(|| true),
            ..Default::default()
        });
        
        let rendered = dialog.render();
        assert!(rendered.contains("role=\"dialog\""));
        assert!(rendered.contains("aria-modal=\"true\""));
    }

    #[test]
    fn test_dialog_aria_labelledby() {
        let content = DialogContent::new(DialogContentProps {
            ..Default::default()
        });
        
        let rendered = content.render();
        assert!(rendered.contains("aria-labelledby=\"dialog-title\""));
    }

    #[test]
    fn test_dialog_aria_describedby() {
        let content = DialogContent::new(DialogContentProps {
            ..Default::default()
        });
        
        let rendered = content.render();
        assert!(rendered.contains("aria-describedby=\"dialog-description\""));
    }

    #[test]
    fn test_dialog_focus_management() {
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(|| true),
            ..Default::default()
        });
        
        let rendered = dialog.render();
        assert!(rendered.contains("tabindex=\"-1\""));
    }
}

Day 5: Keyboard Navigation Tests

File: packages/leptos/dialog/src/tests/keyboard_navigation.rs

#[cfg(test)]
mod keyboard_navigation {
    use super::*;

    #[test]
    fn test_dialog_escape_key_closes() {
        let (open, set_open) = signal(true);
        
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(move || open.get()),
            on_open_change: Some(Callback::new(move |new_open| {
                set_open.set(new_open);
            })),
            ..Default::default()
        });
        
        // Simulate escape key
        let keyboard_event = KeyboardEvent::new("keydown").unwrap();
        keyboard_event.set_key("Escape");
        
        // Test that dialog closes
        assert!(!open.get(), "Dialog should close on escape key");
    }

    #[test]
    fn test_dialog_tab_navigation() {
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(|| true),
            ..Default::default()
        });
        
        let rendered = dialog.render();
        // Test that focusable elements are properly marked
        assert!(rendered.contains("tabindex"));
    }

    #[test]
    fn test_dialog_focus_trap() {
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(|| true),
            ..Default::default()
        });
        
        let rendered = dialog.render();
        // Test that focus trap is implemented
        assert!(rendered.contains("focus-trap"));
    }
}

Phase 3: Integration Testing (Week 3)

Day 1-2: Form Integration Tests

File: packages/leptos/dialog/src/tests/integration.rs

#[cfg(test)]
mod integration {
    use super::*;

    #[test]
    fn test_dialog_with_form() {
        let (open, set_open) = signal(true);
        let form_submitted = signal(false);
        
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(move || open.get()),
            children: Some(Children::new()),
            ..Default::default()
        });
        
        // Test form submission within dialog
        form_submitted.set(true);
        assert!(form_submitted.get(), "Form should submit within dialog");
    }

    #[test]
    fn test_dialog_with_multiple_buttons() {
        let (open, set_open) = signal(true);
        let button_clicked = signal(false);
        
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(move || open.get()),
            children: Some(Children::new()),
            ..Default::default()
        });
        
        // Test multiple buttons within dialog
        button_clicked.set(true);
        assert!(button_clicked.get(), "Buttons should work within dialog");
    }

    #[test]
    fn test_dialog_with_input_fields() {
        let (open, set_open) = signal(true);
        let input_value = signal("".to_string());
        
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(move || open.get()),
            children: Some(Children::new()),
            ..Default::default()
        });
        
        // Test input fields within dialog
        input_value.set("test".to_string());
        assert_eq!(input_value.get(), "test");
    }
}

Day 3-4: Performance Tests

File: packages/leptos/dialog/src/tests/performance.rs

#[cfg(test)]
mod performance {
    use super::*;
    use std::time::Instant;

    #[test]
    fn test_dialog_render_performance() {
        let start = Instant::now();
        let dialog = Dialog::new(DialogProps::default());
        let _rendered = dialog.render();
        let duration = start.elapsed();
        
        assert!(duration.as_millis() < 2, "Dialog render time should be < 2ms");
    }

    #[test]
    fn test_dialog_open_close_performance() {
        let (open, set_open) = signal(false);
        
        let start = Instant::now();
        set_open.set(true);
        set_open.set(false);
        let duration = start.elapsed();
        
        assert!(duration.as_millis() < 1, "Dialog open/close should be < 1ms");
    }

    #[test]
    fn test_dialog_memory_usage() {
        let dialog = Dialog::new(DialogProps::default());
        let _rendered = dialog.render();
        
        // Test that dialog doesn't leak memory
        // This is a basic test - more sophisticated memory testing would be needed
        assert!(true, "Dialog should not leak memory");
    }
}

Day 5: Error Handling Tests

File: packages/leptos/dialog/src/tests/error_handling.rs

#[cfg(test)]
mod error_handling {
    use super::*;

    #[test]
    fn test_dialog_graceful_error_handling() {
        let (open, set_open) = signal(true);
        let error_occurred = signal(false);
        
        let dialog = Dialog::new(DialogProps {
            open: Signal::derive(move || open.get()),
            ..Default::default()
        });
        
        // Test graceful error handling
        error_occurred.set(true);
        assert!(error_occurred.get(), "Should handle errors gracefully");
        assert!(open.get(), "Dialog should remain stable during errors");
    }

    #[test]
    fn test_dialog_missing_context_handling() {
        // Test that dialog handles missing context gracefully
        let dialog = Dialog::new(DialogProps::default());
        let _rendered = dialog.render();
        
        // Should not panic when context is missing
        assert!(true, "Dialog should handle missing context gracefully");
    }
}

📋 Implementation Checklist

Week 1: Critical Accessibility Features

  • Day 1: Add ARIA attributes to DialogContent
  • Day 2: Implement focus management system
  • Day 3: Add keyboard navigation support
  • Day 4: Implement focus trap functionality
  • Day 5: Add escape key handling

Week 2: Comprehensive Testing

  • Day 1: Implement modal behavior tests
  • Day 2: Add accessibility tests
  • Day 3: Create keyboard navigation tests
  • Day 4: Add integration tests
  • Day 5: Implement performance tests

Week 3: Integration & Error Handling

  • Day 1: Form integration tests
  • Day 2: Multiple component integration tests
  • Day 3: Performance benchmarking
  • Day 4: Error handling tests
  • Day 5: Final validation and documentation

Week 4: Validation & Documentation

  • Day 1: Run full test suite
  • Day 2: Performance benchmarking
  • Day 3: Accessibility testing
  • Day 4: Update documentation
  • Day 5: Final validation and sign-off

🎯 Success Metrics

Week 1 Targets

  • ARIA attributes implemented
  • Focus management working
  • Keyboard navigation functional
  • Escape key closes dialog

Week 2 Targets

  • 90% test coverage achieved
  • All test categories implemented
  • Accessibility tests passing
  • Integration tests working

Week 3 Targets

  • Form integration working
  • Performance benchmarks established
  • Error handling implemented
  • Production ready

Week 4 Targets

  • 100% test coverage
  • AAA accessibility compliance
  • Performance targets met
  • Documentation complete

🚨 Risk Mitigation

High-Risk Areas

  1. Focus Management: Complex focus trap implementation
  2. Keyboard Navigation: Event handling complexity
  3. Accessibility: WCAG compliance requirements
  4. Performance: Real-time focus management impact

Mitigation Strategies

  1. Incremental Implementation: Small, testable changes
  2. Comprehensive Testing: Test-driven development
  3. Code Reviews: Peer review for all changes
  4. Rollback Plans: Git branches for each phase

🔧 Tools & Dependencies

Required Dependencies

[dev-dependencies]
wasm-bindgen-test = "0.3"
web-sys = { version = "0.3", features = ["KeyboardEvent", "Element", "Document"] }

Testing Tools

  • Unit Testing: Rust native testing
  • WASM Testing: wasm-bindgen-test
  • Accessibility Testing: Manual testing + automated checks
  • Performance Testing: Custom benchmarking

🚀 Next Steps

  1. Start immediately with ARIA attributes
  2. Implement focus management as priority
  3. Add keyboard navigation for accessibility
  4. Create comprehensive tests for all functionality
  5. Validate production readiness before completion

This remediation plan will transform the Dialog component from a basic, inaccessible modal into a production-ready, fully-accessible dialog system within 4 weeks.


Remediation plan created: September 20, 2025
Target completion: October 18, 2025