search

department-of-veterans-affairs / vets-design-system-documentation

Repository for design.va.gov website
https://design.va.gov
57 stars 63 forks source link

Focus should not leave OMB Component Modal #2207

Closed jeana-adhoc closed 11 months ago

jeana-adhoc commented 1 year ago

Bug Report

What happened

In the 508 audit on form 21-10210, received this note from the 508 office regarding the OMB component.

Focusable components in the content do not receive focus in an order that preserves meaning and operability. The following is an example: On the 21-10210 Lay/Witness Statement | Veterans Affairs screen, the focus is not moved to a place that preserves meaning and operability. After activating View Privacy Act Statement Button, when focus is on the Close Privacy Act Statement modal Button the focus can be moved outside the modal when navigating with shift + tab.

WCAG 2.4.3 - Focus Order

I was able to recreate this in storybook. It never happens when you just tab. But when you shift+tab (reverse tab) the focus leaves the modal. If you try this on the regular modal component, the focus never leaves the modal itself.

What I expected to happen

The focus to never leave the modal

Reproducing

Steps to reproduce:

  1. Open https://design.va.gov/storybook/iframe.html?id=components-va-omb-info--default&viewMode=story
  2. tab with the keyboard to the "Privacy Act Statement" button
  3. Hit enter on the keyboard
  4. See the modal load
  5. Press shift+tab on the keyboard
  6. Observe that the original "view privacy act" button gets the focus. (If you press space or enter while the button is activated, it will close the modal")
  7. While within the modal, if you click the close button, the focus does not show itself around the privacy act button, and the <title> is read, and then it just starts reading down the page.

Urgency

How urgent is this request? Please select the approriate option below and/or provide details

Details

caw310 commented 1 year ago

Hey team! Please add your planning poker estimate with Zenhub @Andrew565 @ataker @harshil1793 @it-harrison @jamigibbs @micahchiang @nickjg231 @powellkerry @rmessina1010 @rsmithadhoc

jeana-adhoc commented 1 year ago

Since opening this ticket, I discovered another issue with the modal. When the modal is closed, focus is lost. The keyboard seems to be below the button, but when you press tab, the phone number in the footer gets the focus indicator. However, if you just close the modal, the screen reader announces the Page . When the modal closes, focus should return to the "Privacy act" button, and the privacy act button should be announced. The correct behavior is displaying properly in the <a href="https://design.va.gov/storybook/iframe.html?id=components-va-modal--default&viewMode=story">modal in storybook</a>, but not in the <a href="https://design.va.gov/storybook/iframe.html?id=components-va-omb-info--default&viewMode=story">omb component in storybook</a>.</p> <p>Here is a video </p> <p><a href="https://github.com/department-of-veterans-affairs/vets-design-system-documentation/assets/117098918/8fda76b7-a412-44c4-afc6-231d8357a702">https://github.com/department-of-veterans-affairs/vets-design-system-documentation/assets/117098918/8fda76b7-a412-44c4-afc6-231d8357a702</a></p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/caw310"><img src="https://avatars.githubusercontent.com/u/72393213?v=4" />caw310</a> commented <strong> 11 months ago</strong> </div> <div class="markdown-body"> <p>cross referencing as could be similar to this issue. <a href="https://github.com/department-of-veterans-affairs/vets-design-system-documentation/issues/2255">2255</a></p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/Andrew565"><img src="https://avatars.githubusercontent.com/u/147893?v=4" />Andrew565</a> commented <strong> 11 months ago</strong> </div> <div class="markdown-body"> <h1>The Problem with Modal Focus Trapping</h1> <p>Currently modals use a "focusin" listener to detect when focus leaves the modal and enters the main body of a document. The Shadow DOM messes up the "focusin" listener, such that it doesn’t fire for anything inside of a Shadow DOM.</p> <p><code>document.querySelector(“:focus”)</code> doesn’t track inside of the shadow DOM, making it difficult to programmatically detect which element is currently focused. Workaround has been to use <code>event.composedPath()</code> and compare with a list of "focusableChidren", which 'double counts' the <code>va-telephone</code> element and its enclosed <code>a</code> tag. This has been used with the <code>keydown</code> event, which works inside of Shadow DOM, but sequential focusing (using the tab key) prevents the .focus() method from working properly, so the focus ends up getting ‘stuck’ on the close button.</p> <p><a href="https://nolanlawson.com/2022/06/14/dialogs-and-shadow-dom-can-we-make-it-accessible/">This article</a> suggests using <code><dialog></code> could work better, but:</p> <ul> <li>it ‘leaks’ focus to the browser address bar (some call this a feature, others call it a bug)</li> <li>would require a rewrite of the modal component</li> <li>Doesn’t support Safari < 15.4</li> </ul> <h2>Options</h2> <ul> <li>Best option is to get rid of shadow dom and rely on the existing “focusin” listener.</li> <li>Second best option would be to add a 'focusin' listener to the OMB-info and CLM "modal triggering buttons" which would redirect focus back to the modal, but may still momentarily read out their contents to screen readers in the process.</li> <li>Third best option is to use a dialog with the known caveats.</li> <li>Fourth best option is to keep things as they are, with a faulty focus trap for OMB-info and Crisis Line Modal (other modals unaffected).</li> </ul> <p>(This was also posted to Slack, will update here when any decisions are made.)</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/Andrew565"><img src="https://avatars.githubusercontent.com/u/147893?v=4" />Andrew565</a> commented <strong> 11 months ago</strong> </div> <div class="markdown-body"> <p>The solution decided on was to attach 'focus' listeners to the modal-triggering buttons for the OMB Info and Crisis Line Modal components, which appears to work well enough for our needs.</p> </div> </div> <div class="comment"> <div class="user"> <a rel="noreferrer nofollow" target="_blank" href="https://github.com/Andrew565"><img src="https://avatars.githubusercontent.com/u/147893?v=4" />Andrew565</a> commented <strong> 11 months ago</strong> </div> <div class="markdown-body"> <p>After receiving more feedback from Ryan, the following work still needs to be done:</p> <ul> <li>[x] Need to apply 'button focus hack' to the minimal header VA logo</li> <li>[x] Need to reconfigure 'button focus hack' and 'initial modal open' behaviors to focus on the 'role="dialog"' element rather than on the close button</li> <li>[x] Need to reconfigure 'button focus hack' to focus on the last element rather than the first element when the modal is shift+tabbed into (this may not be feasible without some significant refactoring of the modal, as the button hack doesn't know about the modal's focusable elements beyond the 'role="dialog"' main element and the close button, and those it only knows about because they're hardcoded into the hack)</li> <li>Firefox doesn't read anything in the modal, a problem which is better addressed by #2255</li> </ul> <p>Because of these additional requirements, I adjusted the point estimate for this ticket from 3 to 5, and expect that it will carry over. CC @humancompanion-usds , @caw310, @micahchiang </p> </div> </div> <div class="page-bar-simple"> </div> <div class="footer"> <ul class="body"> <li>© <script> document.write(new Date().getFullYear()) </script> Githubissues.</li> <li>Githubissues is a development platform for aggregating issues.</li> </ul> </div> <script src="https://cdn.jsdelivr.net/npm/jquery@3.5.1/dist/jquery.min.js"></script> <script src="/githubissues/assets/js.js"></script> <script src="/githubissues/assets/markdown.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/highlight.min.js"></script> <script src="https://cdn.jsdelivr.net/gh/highlightjs/cdn-release@11.4.0/build/languages/go.min.js"></script> <script> hljs.highlightAll(); </script> </body> </html>