🎨 Palette: Add accessibility attributes and focus states to SectionEditor controls#545
🎨 Palette: Add accessibility attributes and focus states to SectionEditor controls#545aafre wants to merge 1 commit into
Conversation
…itor controls 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. |
There was a problem hiding this comment.
Code Review
This pull request enhances the SectionEditor component by adding explicit button types, ARIA labels, and focus-visible styles to improve accessibility and prevent accidental form submissions. It also updates the project's palette documentation to reflect these standards. Review feedback focuses on further improving accessibility by making ARIA labels more context-specific and ensuring focus ring colors align with the established design system.
| className="bg-red-500 text-white px-3 py-1 rounded hover:bg-red-600 focus:outline-none focus-visible:ring-2 focus-visible:ring-red-600 focus-visible:ring-offset-1" | ||
| aria-label={`Remove item ${itemIndex + 1}`} |
There was a problem hiding this comment.
There are three improvements for this button:
- Consistency: The focus ring color should use
focus-visible:ring-accentto align with the repository's design system. - Accessibility: The
aria-labelshould be more descriptive by including the text of the item being removed. - Best Practices: Explicitly set
type='button'to prevent the button from accidentally submitting a form if it's ever placed within a<form>element.
| className="bg-red-500 text-white px-3 py-1 rounded hover:bg-red-600 focus:outline-none focus-visible:ring-2 focus-visible:ring-red-600 focus-visible:ring-offset-1" | |
| aria-label={`Remove item ${itemIndex + 1}`} | |
| type="button" className="bg-red-500 text-white px-3 py-1 rounded hover:bg-red-600 focus:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-1" | |
| aria-label={item.text ? "Remove " + item.text : "Remove item " + (itemIndex + 1)} |
References
- Always add type="button", descriptive aria-labels, and explicit focus classes (focus:outline-none focus-visible:ring-2 focus-visible:ring-accent rounded) to all custom inline editor controls. (link)
- Explicitly set type='button' on elements that are not intended to submit a form to prevent unintended form submissions.
| }} | ||
| className="bg-accent text-ink px-4 py-2 mt-2 rounded hover:bg-accent" | ||
| className="bg-accent text-ink px-4 py-2 mt-2 rounded hover:bg-accent focus:outline-none focus-visible:ring-2 focus-visible:ring-accent focus-visible:ring-offset-1" | ||
| aria-label="Add new item to list" |
There was a problem hiding this comment.
Improve accessibility by including the section name in the aria-label to provide better context for screen reader users. Additionally, ensure the button has an explicit type='button' attribute to prevent unintended form submissions.
| aria-label="Add new item to list" | |
| type="button" aria-label={"Add new item to " + section.name} |
References
- Explicitly set type='button' on elements that are not intended to submit a form to prevent unintended form submissions.
💡 What: Added explicit
type="button", descriptivearia-labels, and focus visibility rings to all action buttons in theSectionEditorcomponent. Wrapped emoji icons in spans withrole="img"andaria-hidden="true".🎯 Why: Prevents accidental form submissions if the editor is ever wrapped in a form, ensures keyboard users can clearly see which control has focus, and provides proper screen reader context for icon-only buttons.
📸 Before/After: No visual changes for mouse users; keyboard users now see clear focus rings around interactive elements.
♿ Accessibility: Added explicit semantic labeling (
aria-label) to icon/emoji-only buttons, hid the raw emojis from screen readers to prevent confusing readouts, and improved keyboard navigation visibility with Tailwind'sfocus-visibleclasses.PR created automatically by Jules for task 5915761666690341594 started by @aafre