🎨 Palette: Enhance input feedback and accessibility#24
🎨 Palette: Enhance input feedback and accessibility#24CassieMarie0728 wants to merge 1 commit intomainfrom
Conversation
- Add character counter and maxLength to chat composer - Link auxiliary descriptions using aria-describedby - Add role="alert" to chat error messages - Add descriptive title to disabled send button - Hide decorative dots in intensity toggle from screen readers Co-authored-by: CassieMarie0728 <[email protected]>
|
👋 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. |
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
Review these changes at https://app.gitnotebooks.com/CassieMarie0728/the-fork/pull/24 |
✅ Deploy Preview for the-fork-chat ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Code Review
This pull request enhances the accessibility of the application by implementing programmatic associations for auxiliary descriptions, adding ARIA roles to error messages, and hiding decorative elements from screen readers. Key changes include the addition of a character counter to the chat composer and the use of aria-describedby for input hints. Review feedback suggests improving the accessibility of the send button's state by avoiding the title attribute, which is inaccessible to keyboard and touch users, and addressing redundant screen reader announcements in the intensity toggle by providing an explicit aria-label to separate the button's name from its description.
| title={ | ||
| draft.trim().length === 0 ? "Write something first" : undefined | ||
| } |
There was a problem hiding this comment.
The title attribute has significant accessibility limitations: it is generally not accessible to keyboard users (who cannot hover) or touch device users. Furthermore, the current logic only provides a description for the "empty" state, leaving the "loading" state unexplained even though the button is disabled in both cases.
Consider using a visible hint or an aria-live region to provide feedback that is accessible to all users. Alternatively, use aria-disabled="true" instead of disabled to keep the button focusable so that the reason for the disabled state can be announced by screen readers.
title={
loading
? "Sending message..."
: draft.trim().length === 0
? "Write something first"
: undefined
}| type="button" | ||
| role="radio" | ||
| aria-checked={active} | ||
| aria-describedby={`intensity-hint-${key}`} |
There was a problem hiding this comment.
Adding aria-describedby while the hint text is a child of the button creates a redundant announcement. By default, screen readers include all nested text content in the button's accessible name. Linking it again via aria-describedby causes many screen readers to announce the hint twice (once as part of the name and once as the description).
To follow the project's goal of reducing noise (as noted in .Jules/palette.md) while maintaining programmatic association, consider providing an explicit aria-label for the button. This ensures the button's name is concise and the hint is correctly treated as a separate description.
| aria-describedby={`intensity-hint-${key}`} | |
| aria-label={meta.label} | |
| aria-describedby={`intensity-hint-${key}`} |
References
- The learning log suggests linking hints via aria-describedby, but in this specific implementation (nested elements), it leads to redundant announcements which conflicts with the goal of reducing screen reader noise. (link)
💡 What:
maxLength={500}on the chat input.titleto the "Say It" button when it's disabled due to empty input.role="alert"to the chat error message for immediate screen reader announcement.IntensityToggleaccessibility by linking hints viaaria-describedbyand hiding decorative dots viaaria-hidden="true".🎯 Why:
♿ Accessibility:
PR created automatically by Jules for task 16888688988936609541 started by @CassieMarie0728