LG-12595 Fix Selfie Screenreader Behavior by adding <FullScreen />#10228
LG-12595 Fix Selfie Screenreader Behavior by adding <FullScreen />#10228
Conversation
…blem by adding FullScreen component
charleyf
requested review from
a team and
night-jellyfish
and removed request for
a team
eileen-nava
left a comment
eileen-nava
left a comment
There was a problem hiding this comment.
This looks good to me. Approved. 👍🏻
| const button = queryByLabelText('account.navigation.close'); | ||
|
|
||
| expect(button).to.not.exist(); | ||
| // The button still exists, but without this class it doesn't show because the |
| type="button" | ||
| aria-label={t('account.navigation.close')} | ||
| onClick={onRequestClose} | ||
| className={hideCloseButton ? '' : 'full-screen__close-button usa-button padding-2 margin-2'} |
There was a problem hiding this comment.
I see the comment above about z-index effectively hiding this button. Does that apply for screen readers as well? My concern is that someone using a screen reader could still find this button by navigating content, but the way we're implementing it seems to imply we don't want it to be available on the page.
I guess I'm not really following why we're making this change at all.
There was a problem hiding this comment.
Ah, thanks. Here's the "why" for this change:
- This is currently the only place in the codebase where
hideCloseButtonis in use. - We don't want to show the close button (gray with an X icon) because it overlaps the "close" text in the top right corner and looks bad on the selfie capture screen.
- If I try to use
hideCloseButtonas it's currently implemented inmain, it does not work for selfie capture (see details below) because the Acuant "close" functionality appears after the focus trap is activated, which makes the focus trap error out. - This is my (inelegant) way of solving the problem of having two overlapping close functionalities and hiding the close button with the "X".
Other thoughts:
- Thinking about this I realized that the hidden close button should continue to work, so I did this.
- I'm open to other solutions, I couldn't come up with anything else that fit within a reasonable scope.
Details on why hideCloseButton doesn't work:
- When I get to the front/back/selfie screen all is well.
- But clicking on the selfie capture box does not open the selfie capture as expected.
- Instead of opening selfie capture I get a console log from the focus-trap library that says something to the effect of "The focus trap needs to have a clickable element at all times".
- I think that this is happening because there's some amount of time where the focus trap is active on
acuant-face-capture-containerbut Acuant hasn't rendered in theclosetext yet.
There was a problem hiding this comment.
I guess one thing I'd want to have clarification on is: if we're intending to hide this, is it also hidden from assistive technology (screen reader)?
There was a problem hiding this comment.
Another good point. I tried another approach with these last commits:
- I reverted the changes to the fullscreen button (commit).
- Instead, I'm doing the same thing that works for the Document capture - putting a button on the screen that we don't ever see, but would allow the user to close the screen (commit).
I think this is significantly better than the earlier idea.
There was a problem hiding this comment.
Yeah that looks a lot better 👍
There was a problem hiding this comment.
🚀 Thanks for all the comments! I really appreciate it.
night-jellyfish
left a comment
night-jellyfish
left a comment
There was a problem hiding this comment.
Tested and saw the expected behavior. 🎉 Thanks for fixing this!
5 participants
🎫 Ticket
https://cm-jira.usa.gov/browse/LG-12595
🛠 Summary of changes
This PR wraps the selfie capture screen (where you see your face to take a pic) in a component to make it work with screenreaders. Before this PR, you could take a selfie, but trying to click the "retake" link button would not work correctly. The screenreader would start reading hidden content behind the capture window. With this PR the hidden content correctly remains unread.
📜 Testing Plan
Provide a checklist of steps to confirm the changes.
mainbranch.Make sure you understand the problem:
Then see that this branch fixes the problem.
charley/lg-12595-add-fullscreen-wrapper-to-selfie