Skip to content

🎨 Palette: Add accessibility enhancements to ResponsiveConfirmDialog#565

Open
aafre wants to merge 1 commit into
mainfrom
palette-a11y-responsive-confirm-dialog-2115728076009702450
Open

🎨 Palette: Add accessibility enhancements to ResponsiveConfirmDialog#565
aafre wants to merge 1 commit into
mainfrom
palette-a11y-responsive-confirm-dialog-2115728076009702450

Conversation

@aafre
Copy link
Copy Markdown
Owner

@aafre aafre commented May 31, 2026

💡 What:

  • Added a useEffect hook to handle the Escape key press and call onClose inside the ResponsiveConfirmDialog component.
  • Added explicit keyboard focus visibility classes (focus:outline-none focus-visible:ring-2 focus-visible:ring-offset-2) to the Close, Cancel, and Confirm buttons.
  • Contextual colors were added for the focus ring on the Confirm button (e.g., focus-visible:ring-red-500 for destructive, focus-visible:ring-accent for standard).

🎯 Why:
Custom modals implemented in React without native <dialog> elements often miss built-in keyboard behaviors. Specifically, they lack automatic handling of the Escape key for closing and custom styled buttons lose default browser focus outlines. This change resolves those issues to improve keyboard navigation and accessibility.

♿ Accessibility:

  • Explicit focus rings provide a visual indicator of keyboard focus for standard and destructive actions within the confirm dialog.
  • The Escape key can now be used to close the modal directly.

PR created automatically by Jules for task 2115728076009702450 started by @aafre

Co-authored-by: aafre <8656674+aafre@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown
Contributor

👋 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 @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +36 to +51
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]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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
  1. For accessibility, all modals must implement a focus trap to ensure keyboard focus remains within the modal's focusable elements while it is open.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant