🎨 Palette: Add accessibility enhancements to ResponsiveConfirmDialog#565
🎨 Palette: Add accessibility enhancements to ResponsiveConfirmDialog#565aafre wants to merge 1 commit into
Conversation
Co-authored-by: aafre <8656674+aafre@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
Code Review
This pull request enhances the accessibility of the ResponsiveConfirmDialog component by adding an Escape key listener to close the dialog and applying visible focus styles to its interactive elements. The reviewer recommends implementing a focus trap to keep keyboard focus within the modal while it is open, providing a detailed code suggestion to achieve this.
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| if (e.key === "Escape" && isOpen) { | ||
| e.preventDefault(); | ||
| onClose(); | ||
| } | ||
| }; | ||
|
|
||
| if (isOpen) { | ||
| document.addEventListener("keydown", handleKeyDown); | ||
| } | ||
|
|
||
| return () => { | ||
| document.removeEventListener("keydown", handleKeyDown); | ||
| }; | ||
| }, [isOpen, onClose]); |
There was a problem hiding this comment.
According to the general rules, all modals must implement a focus trap to ensure keyboard focus remains within the modal's focusable elements while it is open. Since this is a custom modal, we should implement a focus trap to prevent keyboard focus from escaping the dialog when pressing Tab or Shift+Tab, and automatically focus the first element when the dialog opens.
useEffect(() => {
let timeoutId: number;
const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === "Escape" && isOpen) {
e.preventDefault();
onClose();
}
if (e.key === "Tab" && isOpen) {
const dialog = document.querySelector('[role="dialog"][aria-modal="true"]');
if (!dialog) return;
const focusableElements = dialog.querySelectorAll(
"button, [href], input, select, textarea, [tabindex]:not([tabindex='-1'])"
);
if (focusableElements.length === 0) return;
const firstElement = focusableElements[0] as HTMLElement;
const lastElement = focusableElements[focusableElements.length - 1] as HTMLElement;
if (e.shiftKey) {
if (document.activeElement === firstElement) {
lastElement.focus();
e.preventDefault();
}
} else {
if (document.activeElement === lastElement) {
firstElement.focus();
e.preventDefault();
}
}
}
};
if (isOpen) {
document.addEventListener("keydown", handleKeyDown);
timeoutId = window.setTimeout(() => {
const dialog = document.querySelector('[role="dialog"][aria-modal="true"]');
if (dialog) {
const focusableElements = dialog.querySelectorAll(
"button, [href], input, select, textarea, [tabindex]:not([tabindex='-1'])"
);
if (focusableElements.length > 0) {
(focusableElements[0] as HTMLElement).focus();
}
}
}, 0);
}
return () => {
document.removeEventListener("keydown", handleKeyDown);
if (timeoutId) {
window.clearTimeout(timeoutId);
}
};
}, [isOpen, onClose]);
References
- For accessibility, all modals must implement a focus trap to ensure keyboard focus remains within the modal's focusable elements while it is open.
💡 What:
useEffecthook to handle theEscapekey press and callonCloseinside theResponsiveConfirmDialogcomponent.focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2) to the Close, Cancel, and Confirm buttons.focus-visible:ring-red-500for destructive,focus-visible:ring-accentfor standard).🎯 Why:
Custom modals implemented in React without native
<dialog>elements often miss built-in keyboard behaviors. Specifically, they lack automatic handling of theEscapekey for closing and custom styled buttons lose default browser focus outlines. This change resolves those issues to improve keyboard navigation and accessibility.♿ Accessibility:
Escapekey can now be used to close the modal directly.PR created automatically by Jules for task 2115728076009702450 started by @aafre