feat(lit): add accessibility attributes support to Lit basic catalog components (#1410) - #2144
feat(lit): add accessibility attributes support to Lit basic catalog components (#1410)#2144josemontespg wants to merge 10 commits into
Conversation
josemontespg
left a comment
There was a problem hiding this comment.
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
Cardcomponents with accessibility attributes to renderrole="region"along witharia-label(e.g.<div role="region" aria-label="...">). In HTML/WAI-ARIA, a generic<div>witharia-labelrequiresrole="region"to be recognized as a landmark container in screen reader accessibility trees. - Required Fix: Add
role="region"to the rendered container<div>whenlabelordescriptionis 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
Textas one of the required catalog components to supportaccessibility.labelandaccessibility.description.Text.tswas omitted from this PR. - Required Fix: Update
Text.tsto bindaria-labelandaria-describedbywhenprops.accessibilityis 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, andVideoto verify thataria-label,aria-describedby,alt, andaria-hiddenattributes render correctly whenaccessibilityproperties are set.
There was a problem hiding this comment.
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.
|
Adversarial review passed. Ready for Captain LGTM. |
…dal, Slider, and Video
…d Angular basic catalog components (a2ui-project#1410)
13200a1 to
0137247
Compare
josemontespg
left a comment
There was a problem hiding this comment.
Adversarial Reviewer Objections:
- Card Component (React & Angular):
role="region"is missing on Card container<div>whenaria-label/aria-descriptionare provided inrenderers/react/src/v0_9/catalog/basic/components/Card.tsxandrenderers/angular/src/v0_9/catalog/basic/card.component.ts. Per WAI-ARIA standards and approved design,aria-labelon generic<div>elements is ignored by screen readers without a landmark role likerole="region"(as implemented in Lit). - Angular ChoicePicker:
[attr.aria-pressed]="isSelected(option.value)"is missing on chip<button>elements inrenderers/angular/src/v0_9/catalog/basic/choice-picker.component.ts. React implementation correctly setsaria-pressed. - Modal Close Button (React & Angular):
<button class="a2ui-modal-close">renders×withoutaria-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). - Modal Container Semantics (React & Angular): Modal overlay content containers in React (
Modal.tsx) and Angular (modal.component.ts) missrole="dialog"andaria-modal="true". - Angular TextField Label:
<label>in Angulartext-field.component.tslacksforbinding to connect it to<input>(unlike ReactTextField.tsxwhich usesReact.useId()).
3091848 to
de0a749
Compare
de0a749 to
4f99377
Compare
|
Adversarial review passed. React and Angular accessibility fixes match the approved design and all tests pass. |
What
Adds
aria-labelandaria-describedbyattributes support to Lit basic catalog components (Button,CheckBox,ChoicePicker,DateTimeInput,Icon,Image,Slider,TextField,Video,AudioPlayer,Card, andModal).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.labelandprops.accessibility.description) are now rendered asaria-labelandaria-describedbyon Lit basic catalog component elements.