Browser: Chrome (probably any)
Automated testing tool and ruleset: N/A, but IBM DAP surfaces the issue through color contrast rules
Assistive technology used to verify: None & JAWS
What version of the Carbon Design System are you using?
See https://react.carbondesignsystem.com/iframe.html?id=modal--default
What's the issue?
Content within the modal marked as bx--visually-hidden is visible to ATs and copyable text, whether the modal is hidden or not.
Result is:
Focus sentinel
Label
Modal headingPlease see ModalWrapper for more examples and demo of the functionality.
Secondary Button
Primary Button
Focus sentinel
The example doesn't give you a way to hide the modal, but if you do hide the modal, and repeat the steps above, you see:
Focus sentinel
Focus sentinel
Note: The focus sentinel doesn't make a lot of sense to me either, but simply highlights the issue of any content marked with bx--visually-hidden
There are somewhat two issues here:
1) What is the point of "Focus Sentinel"
2) The visibility of the content within the modal after the modal is close. The modal is hidden via CSS visibility: hidden and the bx--visually-hidden uses visibility: visible. For CSS visibility, a visible element within a hidden element is visible. (see https://jakearchibald.com/2014/visible-undoes-hidden/ for an example). display:none is better at hiding all children, but that might affect your animation? It's the safest bet. However, even if you don't go that route, for this particular situation you could do something to override the visibility: visible within a modal, like .modal-hidden .bx--visually-hidden { visibility: hidden } (you don't currently have a .modal-hidden class as far as I can tell though.
Historically a couple different things I've done to address very similar problems:
WH also uses the bx--assistive-text class within Modals as well. This class does the same as bx--visually-hidden so can we make sure both use-cases are addressed with the solution.
possibly related #2740
I believe the fix here is removing the text inside the nodes, correct? Over in: https://github.com/carbon-design-system/carbon/pull/5260/files#diff-2c7f897c16aa710c426b6fd9c3ad155fR241
cc @asudoh
@joshblack Removing the focus sentinel text fixes the example on the URL, but this was brought to my attention by someone using bx-assistive-text / bx--visually-hidden within the modal for other purposes, so that doesn't fix the larger issue.
@tombrunet ah got it, sorry I misread this line:
Content within the modal marked as bx--visually-hidden is visible to ATs and copyable text, whether the modal is hidden or not.
Definitely understand now 👍
bx-assistive-text / bx--visually-hidden is intended to be used on elements that should be read by a screen reader, correct? i.e hidden labels.
I'm not sure there is a proper way for something to receive focus, but then not be read by a screen reader...Focus sentinel was added (I believe) to fix keyboard focus getting lost when the modal was opened.
According to the fourth rule of ARIA, aria-hidden="true" should not be used on a focusable element.
We definitely should be toggling aria-hidden on the modal though so it is not read when closed.
Were the users just looking for a helper to hide something in the modal completely?
Made a quick fix to visibility:visible: #5552
Wrt focus sentinel; It's for focus-wrap behavior that ensures viewport won't lose the focus before we can bring the focus back in modal. More details can be found at: https://developers.google.com/web/fundamentals/accessibility/focus/using-tabindex
The "Focus sentinel" content is a pseudo content, given DAP won't allow empty content there. Let us know if there are any suggestions for better content.
I think this issue has been solved, but @tombrunet please let us know if you think otherwise. Thanks!
@asudoh It looks like this hasn't been released yet and https://vanilla.carbondesignsystem.com/ returns a 403 message. Is there an environment where I can test this without having to run it locally?
Uh Netlify for vanilla has been killed... I got https://vanilla.carbondesignsystem.com/ back up and running with latest master.
Thanks @asudoh. This looks to be resolved @tombrunet. I copied WH's HTML in to the Netlify demo and ran DAP without any color contrast violations.
Most helpful comment
Thanks @asudoh. This looks to be resolved @tombrunet. I copied WH's HTML in to the Netlify demo and ran DAP without any color contrast violations.