Conversation
🦋 Changeset detectedLatest commit: 979dd39 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
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 enhances 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 introduces significant and well-implemented accessibility improvements for the ix-pill component, including transferring ARIA attributes to the Shadow DOM, making icons decorative by default, and adding comprehensive tests and documentation. A security audit confirmed that no vulnerabilities were identified, with the implementation safely handling ARIA attributes and data flow. However, a critical issue was found in packages/html-test-app/src/preview-examples/pill.html where reformatting has led to invalid HTML syntax, which will break the preview and requires immediate attention. This issue is in violation of the rule to ensure HTML is well-formed by removing any extra closing tags to prevent invalid HTML.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request provides a comprehensive set of accessibility improvements for the ix-pill component. The approach of transferring ARIA attributes from the host to an internal element in the Shadow DOM is well-implemented using the a11yHostAttributes utility. The distinction between decorative and informational icons with aria-hidden and aria-label-icon is a great enhancement. The changes are also thoroughly covered by new component tests, accessibility tests (axe-core), and extensive Storybook examples, which is excellent. The updates to the test infrastructure to support custom axe rules per test are also a valuable addition. The minor suggestion to improve adherence to ARIA specifications in a Storybook example remains valid.
packages/storybook-docs/src/stories/pill/accessibility/a11y-pill.stories.ts
Show resolved
Hide resolved
alexkaduk
force-pushed
the
feature/IX-3654-pill-a11y
branch
from
e4e0f29 to
979dd39
Compare
|
1 participant
💡 What is the current behavior?
ARIA attributes (
aria-label,aria-hidden,role, etc.) set on the<ix-pill>host element don't work due to Shadow DOM encapsulation. Icon-only pills have no accessible name (WCAG 1.1.1 violation), and all icons are exposed to screen readers causing redundant announcements. No automated accessibility tests or documentation exists for accessible pill usage patterns.GitHub Issue Number: #3654
🆕 What is the new behavior?
a11yHostAttributesutilityrole="group"inference whenaria-labelis set without explicit rolearia-hidden="true") to prevent redundant screen reader announcementsaria-labelon host element for accessible nameariaLabelIconprop (HTML:aria-label-icon) for informational icons that convey additional meaningrole="status"andaria-live="polite"🏁 Checklist
A pull request can only be merged if all of these conditions are met (where applicable):
pnpm test)pnpm lint)pnpm build, changes pushed)👨💻 Help & support
Breaking Changes: None - all changes are backward compatible.
Action Required for WCAG Compliance:
aria-labelattribute:<ix-pill icon="star" aria-label="Featured"></ix-pill>Testing Note: Pills are non-interactive elements - use browse mode (arrow keys) for screen reader testing, not Tab navigation.