🎨 Palette: [Accessibility] Improve LinkInsertionModal screen reader & keyboard support#383
🎨 Palette: [Accessibility] Improve LinkInsertionModal screen reader & keyboard support#383aafre wants to merge 1 commit into
Conversation
… keyboard support - Add explicit `role="dialog"`, `aria-modal="true"`, and `aria-labelledby` attributes to modal container. - Semantically link `<label>` text to `<input>` tags using explicit `id` and `htmlFor`. - Add `type="button"` and `focus-visible` utility classes to action buttons to ensure clear keyboard navigability. 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. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the accessibility of the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request makes good progress on improving the accessibility of the LinkInsertionModal by adding necessary ARIA roles and properties, associating labels with inputs, and adding visible focus states. However, it's missing a critical accessibility feature for modals: focus trapping. Without it, keyboard users can navigate outside the modal, leading to a confusing experience. I've left a comment with details on how to implement this. I've also suggested updating the associated learnings document to include focus trapping as a requirement for accessible modals.
| role="dialog" | ||
| aria-modal="true" | ||
| aria-labelledby="link-modal-title" |
There was a problem hiding this comment.
While adding role="dialog" and aria-modal="true" is a great step for accessibility, it's incomplete without implementing focus trapping. An accessible modal dialog must prevent keyboard focus from leaving the modal.
When aria-modal="true" is set, assistive technologies expect focus to be contained. Currently, a user can tab out of the modal to the underlying page, which is a confusing experience and fails WCAG guidelines.
To fix this, you need to implement focus trapping. When the modal is open, tabbing should cycle through the modal's interactive elements only. You can achieve this by:
- Finding all focusable elements within the modal.
- On
Tabkeydown, if focus is on the last element, move it to the first. - On
Shift+Tabkeydown, if focus is on the first element, move it to the last.
Consider using a library like focus-trap-react to simplify this, or add the logic manually within a useEffect hook.
|
|
||
| ## 2025-05-18 - LinkInsertionModal Accessibility Findings | ||
| **Learning:** Custom modals like `LinkInsertionModal` were missing explicit screen reader attributes (`role="dialog"`, `aria-modal="true"`, `aria-labelledby`), unlinked form inputs (`id` to `htmlFor`), and explicit `focus-visible` states on their action buttons. This causes screen readers to misidentify the scope and keyboard users to lose track of focus. | ||
| **Action:** When building custom modals, always enforce explicit `role="dialog"`, `aria-modal="true"`, explicitly link `<label>` tags with input `id`s, and add `focus-visible:ring-2` to all action buttons to ensure robust screen reader and keyboard accessibility. |
There was a problem hiding this comment.
This is a great summary of learnings. To make it more complete and ensure future modals are fully accessible, I'd suggest adding 'focus trapping' to the list of required actions.
| **Action:** When building custom modals, always enforce explicit `role="dialog"`, `aria-modal="true"`, explicitly link `<label>` tags with input `id`s, and add `focus-visible:ring-2` to all action buttons to ensure robust screen reader and keyboard accessibility. | |
| **Action:** When building custom modals, always enforce explicit 'role="dialog"', 'aria-modal="true"', trap focus within the modal, explicitly link 'label' tags with input 'id's, and add 'focus-visible:ring-2' to all action buttons to ensure robust screen reader and keyboard accessibility. |
References
- Ensure documentation, such as testing plans, is updated to reflect UI and implementation changes made in the same pull request.
1 participant
💡 What: Improved screen reader and keyboard accessibility for the
LinkInsertionModal.🎯 Why: Custom modals can break accessibility tools if they do not declare themselves as dialogs, fail to link their input fields explicitly to labels, or omit focus rings for keyboard users.
♿ Accessibility:
role="dialog"andaria-modal="true".id/htmlFor.focus-visiblestates on interactive buttons.PR created automatically by Jules for task 17592977304736610285 started by @aafre