feat(accessibility): Implement Issue 1410: Add accessibility attributes to A2UI components - #2141
feat(accessibility): Implement Issue 1410: Add accessibility attributes to A2UI components#2141josemontespg wants to merge 3 commits into
Conversation
…es to A2UI components.
There was a problem hiding this comment.
Code Review
This pull request introduces accessibility attributes, including role, aria-label, and aria-describedby, to basic components (Button, Card, Image, Text, and TextField) in both Angular and Lit renderers, accompanied by schema updates and unit tests. The review feedback suggests using the nullish coalescing operator (??) instead of logical OR (||) for image alt text fallbacks to preserve empty strings for decorative images. Additionally, it recommends refactoring the Angular Button and TextField components to use a computed accessibility signal to avoid duplicate lookups and maintain consistency.
| readonly fit = computed(() => this.props()['fit']?.value() || 'cover'); | ||
| readonly variant = computed(() => this.props()['variant']?.value() || 'default'); | ||
| readonly accessibility = computed(() => this.props()['accessibility']?.value()); | ||
| readonly altText = computed(() => this.accessibility()?.label || this.accessibility()?.description || this.description()); |
There was a problem hiding this comment.
Using the logical OR operator (||) for altText fallback will cause empty strings ("") to be overridden by the fallback descriptions. In HTML/accessibility standards, an empty alt attribute (alt="") is semantically meaningful and indicates a decorative image that screen readers should ignore. Using the nullish coalescing operator (??) preserves empty strings while correctly falling back when the properties are undefined or null.
| readonly altText = computed(() => this.accessibility()?.label || this.accessibility()?.description || this.description()); | |
| readonly altText = computed(() => this.accessibility()?.label ?? this.accessibility()?.description ?? this.description()); |
There was a problem hiding this comment.
Done. Implemented using nullish coalescing to preserve empty strings.
| const altText = | ||
| props.accessibility?.label || | ||
| props.accessibility?.description || | ||
| props.description || | ||
| ''; |
There was a problem hiding this comment.
Using the logical OR operator (||) for altText fallback will cause empty strings ("") to be overridden by the fallback descriptions. In HTML/accessibility standards, an empty alt attribute (alt="") is semantically meaningful and indicates a decorative image that screen readers should ignore. Using the nullish coalescing operator (??) preserves empty strings while correctly falling back when the properties are undefined or null.
| const altText = | |
| props.accessibility?.label || | |
| props.accessibility?.description || | |
| props.description || | |
| ''; | |
| const altText = | |
| props.accessibility?.label ?? | |
| props.accessibility?.description ?? | |
| props.description ?? | |
| ''; |
There was a problem hiding this comment.
Done. Implemented using nullish coalescing.
| readonly ariaLabel = computed(() => this.props()['accessibility']?.value()?.label ?? null); | ||
| readonly ariaDescribedBy = computed(() => this.props()['accessibility']?.value()?.description ?? null); |
There was a problem hiding this comment.
For consistency with other components (like CardComponent, ImageComponent, and TextComponent) and to avoid duplicate lookups of this.props()['accessibility']?.value(), define a computed accessibility signal and use it to resolve ariaLabel and ariaDescribedBy.
| readonly ariaLabel = computed(() => this.props()['accessibility']?.value()?.label ?? null); | |
| readonly ariaDescribedBy = computed(() => this.props()['accessibility']?.value()?.description ?? null); | |
| readonly accessibility = computed(() => this.props()['accessibility']?.value()); | |
| readonly ariaLabel = computed(() => this.accessibility()?.label ?? null); | |
| readonly ariaDescribedBy = computed(() => this.accessibility()?.description ?? null); |
There was a problem hiding this comment.
Done. Optimized accessibility lookup with a computed signal.
| readonly ariaLabel = computed(() => this.props()['accessibility']?.value()?.label ?? null); | ||
| readonly ariaDescribedBy = computed(() => this.props()['accessibility']?.value()?.description ?? null); |
There was a problem hiding this comment.
For consistency with other components (like CardComponent, ImageComponent, and TextComponent) and to avoid duplicate lookups of this.props()['accessibility']?.value(), define a computed accessibility signal and use it to resolve ariaLabel and ariaDescribedBy.
| readonly ariaLabel = computed(() => this.props()['accessibility']?.value()?.label ?? null); | |
| readonly ariaDescribedBy = computed(() => this.props()['accessibility']?.value()?.description ?? null); | |
| readonly accessibility = computed(() => this.props()['accessibility']?.value()); | |
| readonly ariaLabel = computed(() => this.accessibility()?.label ?? null); | |
| readonly ariaDescribedBy = computed(() => this.accessibility()?.description ?? null); |
There was a problem hiding this comment.
Done. Optimized accessibility lookup with a computed signal.
…ize computed accessibility lookups
…for text descriptions
What
This PR implements accessibility attribute mapping for the basic catalog components:
Button,Image,TextField,Card, andText.Specifically, it maps:
aria-label(mapped fromlabel)aria-describedby(mapped fromdescription)role(mapped fromroleinsideaccessibility)It updates both Lit and Angular renderers:
setAttributein theupdated()lifecycle or inline template bindings where applicable.It also adds unit tests for both Lit and Angular to verify the correct attribute mapping.
Why
To address Issue 1410 and ensure that A2UI components render standard ARIA attributes correctly when provided in the component properties.
Effect
Assistive technologies will now be able to read accessibility attributes from these basic components.