-
Notifications
You must be signed in to change notification settings - Fork 393
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: add missing role attribute #19870
base: develop
Are you sure you want to change the base?
Conversation
spartacus Run #46924
Run Properties:
|
Project |
spartacus
|
Branch Review |
fix/CXSPA-9015
|
Run status |
Passed #46924
|
Run duration | 11m 58s |
Commit |
6dc49627c5 ℹ️: Merge 911523e3b596702c65e8decf79232f9e15799987 into 53ca163c60e2124d0ef8369d0297...
|
Committer | sdrozdsap |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
3
|
Pending |
2
|
Skipped |
0
|
Passing |
125
|
Upgrade your plan to view test results. | |
View all changes introduced in this branch ↗︎ |
<div aria-live="assertive" aria-atomic="true"> | ||
<span *ngIf="validationError" tabindex="0" class="cx-invalid-message"> | ||
<span | ||
*ngIf="validationError" | ||
role="alert" | ||
tabindex="0" | ||
class="cx-invalid-message" | ||
> | ||
{{ validationError | cxTranslate }} | ||
</span> | ||
</div> |
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 can remove the aria-live="assertive" aria-atomic="true"
since role alert provides the same functionality.
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.
Since *ngIf and role="alert" doesn't really fit to each other - if we create an element with role alert it is not announced, only when it content changes.
So for that container div has <div aria-live="assertive" aria-atomic="true">
. And role alert on the span is just to suppress AMP warnings
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.
Do you know if there is better way that would work?
No description provided.