🎨 Palette: Improve notification close button accessibility and focus styles#954
🎨 Palette: Improve notification close button accessibility and focus styles#954
Conversation
…styles - Replaced `:focus` with `:focus-visible` for notification buttons to reduce visual noise for mouse users. - Added `title="Close"` and `aria-hidden="true"` to the decorative '✕' character to improve screen reader experience and provide a native tooltip. Co-authored-by: anchapin <6326294+anchapin@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. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImproves the notification close button’s accessibility and focus UX by adding a native tooltip, hiding the decorative “✕” from assistive tech, and updating focus styles to use :focus-visible. Also documents these accessibility learnings in the Palette guide. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider adding a graceful fallback for browsers that don’t support
:focus-visible(e.g., using:focus:not(:focus-visible)or a polyfill) so keyboard users still get a visible focus outline everywhere. - The
title="Close"string is currently hardcoded; if the rest of the UI is localized, it would be better to source this from your i18n layer to keep tooltip text consistent across languages.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider adding a graceful fallback for browsers that don’t support `:focus-visible` (e.g., using `:focus:not(:focus-visible)` or a polyfill) so keyboard users still get a visible focus outline everywhere.
- The `title="Close"` string is currently hardcoded; if the rest of the UI is localized, it would be better to source this from your i18n layer to keep tooltip text consistent across languages.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR improves the accessibility and keyboard-focus styling of the notification close button in the frontend notification/toast system.
Changes:
- Adds a native tooltip (
title) and hides the decorative “✕” from screen readers viaaria-hidden. - Updates notification button focus styling to use
:focus-visibleinstead of:focus. - Records the accessibility learning in the project’s palette log.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/components/NotificationSystem/NotificationSystem.tsx | Updates the close button markup for improved screen reader behavior and tooltip support. |
| frontend/src/components/NotificationSystem/NotificationSystem.css | Adjusts focus styles to be keyboard-intent-based via :focus-visible. |
| .Jules/palette.md | Documents the accessibility rationale and styling guidance. |
| .notification-action-btn:focus-visible, | ||
| .notification-close-btn:focus-visible { | ||
| outline: 2px solid #667eea; | ||
| outline-offset: 2px; |
There was a problem hiding this comment.
Switching these component-specific styles to :focus-visible doesn't prevent focus rings on mouse click because a global button:focus rule still applies (see frontend/src/index.css where button:focus sets an outline). As a result, .notification-*-btn will still show an outline on mouse focus in browsers where :focus-visible is not active. Consider adding a .notification-*-btn:focus:not(:focus-visible) rule (or an @supports selector(:focus-visible) block) to suppress mouse-focus outlines while preserving a keyboard-focus fallback.
| .notification-action-btn:focus-visible, | |
| .notification-close-btn:focus-visible { | |
| outline: 2px solid #667eea; | |
| outline-offset: 2px; | |
| @supports selector(:focus-visible) { | |
| .notification-action-btn:focus-visible, | |
| .notification-close-btn:focus-visible { | |
| outline: 2px solid #667eea; | |
| outline-offset: 2px; | |
| } | |
| .notification-action-btn:focus:not(:focus-visible), | |
| .notification-close-btn:focus:not(:focus-visible) { | |
| outline: none; | |
| } |
| className="notification-close-btn" | ||
| onClick={handleRemove} | ||
| aria-label="Close notification" | ||
| title="Close" |
There was a problem hiding this comment.
The title tooltip text (“Close”) is inconsistent with the accessible name (aria-label="Close notification"). For consistency (and to avoid potentially confusing mixed announcements in some assistive tech), consider aligning the title value with the aria-label (or dropping title if you don’t want a tooltip).
| title="Close" | |
| title="Close notification" |
| ## 2024-06-13 - [Accessibility] Screen readers pronouncing decorative emojis | ||
| **Learning:** Screen readers often misinterpret standalone emojis (e.g., ⬇️, 🗑️) inside buttons, reading them literally, which creates redundant or confusing announcements when a descriptive `aria-label` is already present. | ||
| **Action:** Always wrap non-informational emojis or text-based icons in `<span aria-hidden="true">` when the parent interactive element already has a sufficiently descriptive label. | ||
| ## 2025-04-03 - Improved Notification Close Button Accessibility and Focus Styles |
There was a problem hiding this comment.
This new palette entry date looks off relative to the PR date (<current_datetime> is 2026-04-03). If this entry is meant to reflect this change, update the heading date accordingly to avoid a misleading historical log.
| ## 2025-04-03 - Improved Notification Close Button Accessibility and Focus Styles | |
| ## 2026-04-03 - Improved Notification Close Button Accessibility and Focus Styles |
…styles - Replaced `:focus` with `:focus-visible` for notification buttons to reduce visual noise for mouse users. - Added `title="Close"` and `aria-hidden="true"` to the decorative '✕' character to improve screen reader experience and provide a native tooltip. - Formatted modified files using pnpm prettier. Co-authored-by: anchapin <6326294+anchapin@users.noreply.github.com>
💡 What:
Improved the accessibility and focus styling of the notification close button. Added a
titleattribute for native tooltips, wrapped the decorative "✕" character in anaria-hiddenspan to prevent redundant screen reader announcements, and updated the CSS to use:focus-visibleinstead of:focus.🎯 Why:
To provide a better micro-UX for all users. Mouse users will no longer see unnecessary focus rings when clicking notification buttons, and screen reader users will have a cleaner experience without redundant character announcements. The native tooltip provides immediate clarity for visual users.
♿ Accessibility:
:focus-visible).PR created automatically by Jules for task 1325480363428169002 started by @anchapin
Summary by Sourcery
Improve accessibility and focus behavior for the notification close button and document the pattern in the Palette guidelines.
Enhancements: