Skip to content

🎨 Palette: Add accessibility attributes and focus states to SectionEditor controls#545

Open
aafre wants to merge 1 commit into
mainfrom
palette/section-editor-a11y-5915761666690341594
Open

🎨 Palette: Add accessibility attributes and focus states to SectionEditor controls#545
aafre wants to merge 1 commit into
mainfrom
palette/section-editor-a11y-5915761666690341594

Conversation

@aafre
Copy link
Copy Markdown
Owner

@aafre aafre commented May 22, 2026

💡 What: Added explicit type="button", descriptive aria-labels, and focus visibility rings to all action buttons in the SectionEditor component. Wrapped emoji icons in spans with role="img" and aria-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's focus-visible classes.


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

…itor controls

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 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.

Comment on lines +164 to +165
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}`}
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.

medium

There are three improvements for this button:

  1. Consistency: The focus ring color should use focus-visible:ring-accent to align with the repository's design system.
  2. Accessibility: The aria-label should be more descriptive by including the text of the item being removed.
  3. Best Practices: Explicitly set type='button' to prevent the button from accidentally submitting a form if it's ever placed within a <form> element.
Suggested change
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
  1. 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)
  2. 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"
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.

medium

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.

Suggested change
aria-label="Add new item to list"
type="button" aria-label={"Add new item to " + section.name}
References
  1. Explicitly set type='button' on elements that are not intended to submit a form to prevent unintended form submissions.

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