fix(ui5-button): announce accessible name #11396
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes an accessibility issue for the ui5-button by updating how the accessible name and badge description are announced. Key changes include:
- Updating the aria-label generation by combining the accessible name, button type, and badge description.
- Adjusting the aria-description handling to return undefined when empty.
- Modifying the Cypress tests to validate the updated accessibility attributes.
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/main/src/Button.ts | Updates aria-label and badge description generation for improved announcement. |
| packages/main/cypress/specs/Button.cy.tsx | Updates tests to reflect new accessibility attribute behaviors. |
Files not reviewed (1)
- packages/main/src/i18n/messagebundle.properties: Language not supported
Comments suppressed due to low confidence (1)
packages/main/cypress/specs/Button.cy.tsx:332
- Consider adding additional tests for edge cases such as badge values like "0" or unexpected non-numeric texts to ensure that the aria-label formation handles all scenarios robustly.
.should("have.attr", "aria-label", `Download 999+ items`);
| const internalLabelText = `${typeLabelText} ${this.effectiveBadgeDescriptionText}`.trim(); | ||
|
|
||
| return `${ariaLabelText} ${internalLabelText}`.trim(); |
There was a problem hiding this comment.
[nitpick] Consider handling cases where both the effective aria label and button type text are empty to avoid potential extra spaces, even though trim() is used; a more modular construction of the final label might improve future maintenance.
| const internalLabelText = `${typeLabelText} ${this.effectiveBadgeDescriptionText}`.trim(); | |
| return `${ariaLabelText} ${internalLabelText}`.trim(); | |
| const internalLabelText = this.effectiveBadgeDescriptionText || ""; | |
| // Combine non-empty components into an array and join with a space | |
| const labelParts = [ariaLabelText, typeLabelText, internalLabelText].filter(part => part); | |
| return labelParts.join(" "); |
Copilot uses AI. Check for mistakes.
| return ""; | ||
| } | ||
|
|
||
| const badgeEffectiveText = this.badge[0].effectiveText; | ||
|
|
There was a problem hiding this comment.
[nitpick] Adding a comment to explain the rationale for using distinct i18n keys (BUTTON_BADGE_ONE_ITEM vs. BUTTON_BADGE_MANY_ITEMS) for singular and plural badge values would help improve code clarity.
| // Use distinct i18n keys for singular and plural badge values to ensure proper localization. | |
| // Some languages have different grammatical rules for singular and plural forms, | |
| // so separate keys (BUTTON_BADGE_ONE_ITEM and BUTTON_BADGE_MANY_ITEMS) are necessary. |
Copilot uses AI. Check for mistakes.
| @@ -304,28 +304,47 @@ describe("Accessibility", () => { | |||
| .should("have.attr", "role", "button"); | |||
| }); | |||
|
|
|||
| it("aria-describedby properly applied on the button tag", () => { | |||
| cy.mount(<Button design="Attention">Content</Button>); | |||
| it("accessibleDescription in combination with design property applied on the button tag", () => { | |||
There was a problem hiding this comment.
[nitpick] Ensure that the test description accurately reflects the new behavior, verifying that the aria-description is set as expected for different design scenarios.
| it("accessibleDescription in combination with design property applied on the button tag", () => { | |
| it("verifies that accessibleDescription is correctly applied as aria-description on the button tag, in combination with the design property", () => { |
Copilot uses AI. Check for mistakes.
Issue:
accessible-nameattribute is providedfor the
ui5-button. The reason behind this behaviour is that the screen reader doesn'tread the inner text nodes of the root
buttonnode when the same hasaria-labelattribute.Solution:
aria-labelattribute of the rootbuttonelement. As a result the screen reader won't rely on the inner text contentfor the announcements.
designdescription is also moved into thearia-labelattribute,in order to keep a proper announcement order.
Fixes: #11350