-
Notifications
You must be signed in to change notification settings - Fork 7
Inline Alert accessibility improvements #1105
Conversation
🦋 Changeset detectedLatest commit: 02866b4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@final | ||
export default class ButtonGroupButton extends LitElement { | ||
/* c8 ignore start */ | ||
/* c8 ignore start */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bonus.
2f93045
to
5f89bb0
Compare
src/inline-alert.ts
Outdated
override render() { | ||
return html` | ||
<div | ||
aria-label=${this.variant} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we translate this for screen reader announcements?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We definitely should. Good catch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
✅ f7ff2cd
b2c0f5d
to
f7ff2cd
Compare
🚀 Description
📋 Checklist
🔬 Manual Testing
You know what to do!