Skip to content

🎨 Palette: Improve Duplicate Resume modal accessibility and UX#554

Open
aafre wants to merge 1 commit into
mainfrom
palette/duplicate-resume-modal-a11y-5509710489219307310
Open

🎨 Palette: Improve Duplicate Resume modal accessibility and UX#554
aafre wants to merge 1 commit into
mainfrom
palette/duplicate-resume-modal-a11y-5509710489219307310

Conversation

@aafre
Copy link
Copy Markdown
Owner

@aafre aafre commented May 27, 2026

What: Added standard accessibility attributes and UX interactions to the Duplicate Resume modal.
Why: Custom modals need explicit ARIA roles and keyboard handling to be usable by screen readers and power users.
Before/After: The modal now supports clicking outside to close, pressing Escape to close, and includes appropriate dialog roles for assistive technologies.
Accessibility: Added role="dialog", aria-modal="true", and aria-labelledby, along with Escape key and backdrop click handling.


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

Added standard accessibility attributes and UX interactions to the Duplicate Resume modal to make it usable by screen readers and power users.

- Added role="dialog", aria-modal="true", and aria-labelledby
- Added Escape key listener to close the modal
- Added backdrop click handling to close the modal

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 DuplicateResumeModal by adding ARIA attributes, backdrop click-to-close behavior, and Escape key handling, alongside a learning log entry. The reviewer recommends optimizing the keydown event listener to only register when the modal is open and implementing a focus trap to keep keyboard focus within the modal.

Comment on lines +27 to +35
useEffect(() => {
const handleKeyDown = (e: KeyboardEvent) => {
if (e.key === 'Escape' && isOpen) {
onCancel();
}
};
window.addEventListener('keydown', handleKeyDown);
return () => window.removeEventListener('keydown', handleKeyDown);
}, [isOpen, onCancel]);
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

To improve performance and comply with accessibility standards, we should optimize the keydown event listener and implement a focus trap. Currently, the global keydown listener is registered even when the modal is closed, which is inefficient. Additionally, according to our accessibility guidelines, all modals must implement a focus trap to ensure keyboard focus remains within the modal's focusable elements while it is open.

  useEffect(() => {
    if (!isOpen) return;
    const handleKeyDown = (e: KeyboardEvent) => {
      if (e.key === 'Escape') {
        onCancel();
        return;
      }
      if (e.key === 'Tab') {
        const modal = document.querySelector('[role="dialog"]');
        if (!modal) return;
        const focusable = modal.querySelectorAll(
          'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])"
        );
        if (focusable.length === 0) return;
        const first = focusable[0] as HTMLElement;
        const last = focusable[focusable.length - 1] as HTMLElement;
        if (e.shiftKey) {
          if (document.activeElement === first) {
            last.focus();
            e.preventDefault();
          }
        } else {
          if (document.activeElement === last) {
            first.focus();
            e.preventDefault();
          }
        }
      }
    };
    window.addEventListener('keydown', handleKeyDown);
    return () => window.removeEventListener('keydown', handleKeyDown);
  }, [isOpen, onCancel]);
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