Skip to content

Add checkmark accessibility properties and update documentation#2472

Open
amir-ba wants to merge 2 commits intotelekom:mainfrom
amir-ba:feature/issue-2446
Open

Add checkmark accessibility properties and update documentation#2472
amir-ba wants to merge 2 commits intotelekom:mainfrom
amir-ba:feature/issue-2446

Conversation

@amir-ba
Copy link
Collaborator

@amir-ba amir-ba commented Mar 5, 2026

Summary
This PR enhances the Chip component's accessibility by introducing two new optional properties that allow customization of the checkmark icon's accessibility attributes for persistent chips. fixes #2446

Changes
New Properties:

checkmarkAccessibilityTitle: Allows customization of the checkmark icon's accessibility title (default: 'success')
checkmarkDecorative: Enables hiding the checkmark icon from screen readers when set to true (default: false)
Updated Files:

chip.tsx: Added new @prop() decorators and updated the checkmark icon rendering to use the customizable properties
chip.spec.ts: Added test case to verify custom checkmark accessibility attributes work correctly
readme.md: Updated component documentation with new property descriptions
Chip.stories.mdx: Updated Storybook stories to expose the new properties for testing

Benefits
Improves accessibility customization for different use cases and languages
Allows developers to mark checkmarks as decorative when they're redundant with other visual/textual indicators
Maintains backward compatibility with sensible defaults ('success' title, not decorative)

Testing
Unit test added to verify custom accessibility attributes are properly applied to the checkmark icon
Feel free to adjust the tone or level of detail based on your project's conventions!

Copilot AI review requested due to automatic review settings March 5, 2026 15:37
@amir-ba amir-ba requested a review from maomaoZH as a code owner March 5, 2026 15:37
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR enhances the scale-chip component’s accessibility customization by adding new optional props to control the checkmark icon’s accessible title and whether it should be hidden from screen readers.

Changes:

  • Added checkmarkAccessibilityTitle and checkmarkDecorative props to scale-chip and wired them into the checkmark icon rendering.
  • Added a unit test validating custom accessibility attributes on the checkmark icon.
  • Updated generated docs and Storybook stories to expose/document the new props.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
packages/storybook-vue/stories/components/chip/Chip.stories.mdx Exposes the new checkmark accessibility props in Storybook stories/argTypes.
packages/components/src/components/chip/readme.md Documents the two new optional properties for the chip component.
packages/components/src/components/chip/chip.tsx Adds new props and applies them to the checkmark icon rendering.
packages/components/src/components/chip/chip.spec.ts Adds a unit test covering custom checkmark accessibility attributes.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment on lines +14 to +15
| `checkmarkAccessibilityTitle` | `checkmark-accessibility-title` | (optional) Checkmark icon accessibility title | `string` | `'success'` |
| `checkmarkDecorative` | `checkmark-decorative` | (optional) If `true` the checkmark icon is hidden from screen readers | `boolean` | `false` |
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

The new checkmark properties are documented as general chip props, but they only have an effect for type="persistent" (and only when the checkmark icon is rendered). Update the property descriptions to explicitly state the scope (persistent chips) to avoid misleading consumers.

Copilot uses AI. Check for mistakes.
Comment on lines 128 to 145
} else if (this.type === 'persistent' && this.selected) {
return (
<scale-icon-action-checkmark
accessibility-title="success"
accessibility-title={this.checkmarkAccessibilityTitle}
decorative={this.checkmarkDecorative}
size={16}
selected
/>
);
} else if (this.type === 'persistent') {
return (
<scale-icon-action-checkmark accessibility-title="success" size={16} />
<scale-icon-action-checkmark
accessibility-title={this.checkmarkAccessibilityTitle}
decorative={this.checkmarkDecorative}
size={16}
/>
);
}
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

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

getIcon() contains an else if (this.type === 'persistent') branch that is unreachable with the current render logic (render() only calls getIcon() when this.selected is true). This dead branch increases complexity and can confuse future changes; either remove the unreachable branch or change render() to call getIcon() in the cases where you intend a non-selected persistent icon to be shown.

Copilot uses AI. Check for mistakes.
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.

Accessibility | Chip component - not possible to update accessibility-title for inner component "checkmark" icon or hide for screen readers

2 participants