-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(aria-describer): messages not being read out in IE and Edge #12304
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
Conversation
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.
LGTM
// We only disable `aria-hidden` for these platforms, because it comes with the | ||
// disadvantage that people might hit the messages when they've navigated past | ||
// the end of the document using the arrow keys. | ||
messagesContainer.setAttribute('aria-hidden', canBeAriaHidden + ''); |
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.
Normally I think we do canBeAriaHidden.toString()
? Wouldn't that work as well? seems a bit more consistent with the approach we also use for setting the aria-disabled
state on other components.
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.
I prefer `${canBeAriaHidden}`
but don't really care that much
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.
LGTM. Just an optional nit.
d4fe09a
to
f4b4bf5
Compare
f4b4bf5
to
9964648
Compare
9964648
to
498db46
Compare
498db46
to
a187f26
Compare
a187f26
to
80192c9
Compare
80192c9
to
9f98064
Compare
9f98064
to
46bcd47
Compare
46bcd47
to
a44322f
Compare
a44322f
to
abdb461
Compare
Bumping this to a P2 since this could make some components unusable on IE/Edge with assistive technology. I also spent some time getting the CI to pass again. |
Fixes the messages from the `AriaDescriber` not being read out in Edge or IE, because the message container is `aria-hidden` and has `display: none`. Fixes angular#12298.
abdb461
to
7ace81c
Compare
Fixes the messages from the `AriaDescriber` not being read out in Edge or IE, because the message container is `aria-hidden` and has `display: none`. Fixes #12298.
…lar#12304) Fixes the messages from the `AriaDescriber` not being read out in Edge or IE, because the message container is `aria-hidden` and has `display: none`. Fixes angular#12298.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fixes the messages from the
AriaDescriber
not being read out in Edge or IE, because the message container isaria-hidden
and hasdisplay: none
.Fixes #12298.