Skip to content

feat(lit): add accessibility attributes support to Lit basic catalog components (#1410) - #2144

Open
josemontespg wants to merge 10 commits into
a2ui-project:mainfrom
josemontespg:agent/wt_73962c30
Open

feat(lit): add accessibility attributes support to Lit basic catalog components (#1410)#2144
josemontespg wants to merge 10 commits into
a2ui-project:mainfrom
josemontespg:agent/wt_73962c30

Conversation

@josemontespg

Copy link
Copy Markdown
Collaborator

What

Adds aria-label and aria-describedby attributes support to Lit basic catalog components (Button, CheckBox, ChoicePicker, DateTimeInput, Icon, Image, Slider, TextField, Video, AudioPlayer, Card, and Modal).

Why

Fixes Issue #1410. Ensures components in the Lit renderer properly expose accessibility labels and descriptions to assistive technologies, satisfying WCAG 2.4.6 guidelines.

Effect

Accessibility properties defined in A2UI message schemas (props.accessibility.label and props.accessibility.description) are now rendered as aria-label and aria-describedby on Lit basic catalog component elements.

@josemontespg josemontespg left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial Reviewer Objections for PR #2144 (Issue #1410)

The implementation in @a2ui/lit has been reviewed against the approved design proposal (Issue #1410 Design - Basic Catalog Accessibility Attributes). The following objections must be addressed before approval:

1. Card.ts missing role="region" landmark binding

  • Location: renderers/lit/src/v0_9/catalogs/basic/components/Card.ts
  • Issue: The approved design contract specifically requires Card components with accessibility attributes to render role="region" along with aria-label (e.g. <div role="region" aria-label="...">). In HTML/WAI-ARIA, a generic <div> with aria-label requires role="region" to be recognized as a landmark container in screen reader accessibility trees.
  • Required Fix: Add role="region" to the rendered container <div> when label or description is present.

2. Text.ts missing accessibility attribute bindings

  • Location: renderers/lit/src/v0_9/catalogs/basic/components/Text.ts
  • Issue: The approved design document explicitly identified Text as one of the required catalog components to support accessibility.label and accessibility.description. Text.ts was omitted from this PR.
  • Required Fix: Update Text.ts to bind aria-label and aria-describedby when props.accessibility is provided.

3. Incomplete Unit Test Coverage

  • Location: renderers/lit/src/v0_9/tests/components/
  • Issue: 12 components were updated with accessibility bindings, but unit tests were only added for Button.test.ts.
  • Required Fix: Add unit tests for Card, Image, TextField, Icon, CheckBox, ChoicePicker, Slider, Modal, AudioPlayer, and Video to verify that aria-label, aria-describedby, alt, and aria-hidden attributes render correctly when accessibility properties are set.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request adds accessibility attributes (aria-label and aria-describedby) to various Lit basic catalog components and includes corresponding unit tests. The review feedback highlights critical accessibility issues across multiple components: specifically, aria-describedby is incorrectly used with plain text instead of element IDs and should be replaced with aria-description. Additionally, generic elements like div and span require appropriate semantic roles (such as role="region", role="group", or role="img") to be properly announced by screen readers, and redundant aria-label attributes on <img> elements should be removed.

Comment thread renderers/lit/src/v0_9/catalogs/basic/components/AudioPlayer.ts Outdated
Comment thread renderers/lit/src/v0_9/catalogs/basic/components/Button.ts Outdated
Comment thread renderers/lit/src/v0_9/catalogs/basic/components/Card.ts
Comment thread renderers/lit/src/v0_9/catalogs/basic/components/ChoicePicker.ts
Comment thread renderers/lit/src/v0_9/catalogs/basic/components/Icon.ts Outdated
Comment thread renderers/lit/src/v0_9/catalogs/basic/components/Image.ts Outdated
@josemontespg

Copy link
Copy Markdown
Collaborator Author

Adversarial review passed. Ready for Captain LGTM.

@josemontespg josemontespg left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adversarial Reviewer Objections:

  1. Card Component (React & Angular): role="region" is missing on Card container <div> when aria-label / aria-description are provided in renderers/react/src/v0_9/catalog/basic/components/Card.tsx and renderers/angular/src/v0_9/catalog/basic/card.component.ts. Per WAI-ARIA standards and approved design, aria-label on generic <div> elements is ignored by screen readers without a landmark role like role="region" (as implemented in Lit).
  2. Angular ChoicePicker: [attr.aria-pressed]="isSelected(option.value)" is missing on chip <button> elements in renderers/angular/src/v0_9/catalog/basic/choice-picker.component.ts. React implementation correctly sets aria-pressed.
  3. Modal Close Button (React & Angular): <button class="a2ui-modal-close"> renders &times; without aria-label="Close" in both React (Modal.tsx) and Angular (modal.component.ts), causing screen readers to announce "multiplication sign" or unlabelled button (WCAG 4.1.2).
  4. Modal Container Semantics (React & Angular): Modal overlay content containers in React (Modal.tsx) and Angular (modal.component.ts) miss role="dialog" and aria-modal="true".
  5. Angular TextField Label: <label> in Angular text-field.component.ts lacks for binding to connect it to <input> (unlike React TextField.tsx which uses React.useId()).

@josemontespg

Copy link
Copy Markdown
Collaborator Author

Adversarial review passed. React and Angular accessibility fixes match the approved design and all tests pass.

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