Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RE: Test coverage @aduth. Unfortunately, we still do not have a way to actually get the Acuant SDK to start under any sort of testing conditions. It's a hole we've been aware of for a while, but it's pretty difficult to address.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only see code which expects a DOM element with a particular ID, and a state updater. I don't know that we need a full Acuant installation loaded to be able to test against that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test, which I think does a fairly good job showing how this functionality works.

Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,6 @@ function AcuantCapture(
});

setImageCaptureText('');
setIsCapturingEnvironment(true);
Comment thread
charleyf marked this conversation as resolved.
}

function onSelfieCaptureClosed() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,16 @@ function AcuantSelfieCaptureCanvas({ imageCaptureText, onSelfieCaptureClosed })
// The Acuant SDK script AcuantPassiveLiveness attaches to whatever element has
// this id. It then uses that element as the root for the full screen selfie capture
const acuantCaptureContainerId = 'acuant-face-capture-container';

// This solves a fairly nasty bug for screenreader users where the screenreader focus would jump away
// from the capture button (added by Acuant SDK) to the button in this component. Specifically we
// need to detect when Acuant actually hydrates in their capture screen and hide the button.
// See PR 10668 for more information.
const elementInShadow = document
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Look for the camera container to see if Acuant has actually loaded their stuff onto the screen. Obviously inelegant, but does the job.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this update if Acuant becomes loaded? Are we expecting it to?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's what I think is happening:

  1. <AcuantSelfieCamera> is created, and Acuant is "started", but crucially the capture window doesn't appear anywhere yet.
  2. <FullScreen> activates a focus trap around <AcuantSelfieCaptureCanvas>.
  3. At this moment there's nothing inside that focus trap except a button and a div because Acuant hasn't actually attached to the div with acuantCaptureContainerId.
  4. Then, Acuant attaches to the div with acuantCaptureContainerId and hydrates in their Shadow DOM.

The problem that this PR solves is that it lets me show the button on Step 3, and hide it on Step 4. Practically, what happens if you try to fully remove the button is you get an error about the FocusTrap always needing a tabbable element (which makes sense).

  • So that close button needs to be present until the moment that Acuant's Shadow DOM gets attached.
  • Having that button seems to be the cause of some odd behavior when using a screenreader so it's worthwhile to remove it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The primary concern I have here is that I don't see where we predictably expect React to re-render this component after the changes you describe.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A good point, the re-render isn't necessarily tied to this code in the way I want. We do re-render the AcuantSelfieCaptureCanvas every time the help text changes (frequently) so this value does get updated.

?.getElementById('acuant-face-capture-camera')
?.shadowRoot?.getElementById('cameraContainer');
const loadedAcuantCamera = !!elementInShadow;

return (
<>
{!isReady && <LoadingSpinner />}
Expand All @@ -31,9 +41,11 @@ function AcuantSelfieCaptureCanvas({ imageCaptureText, onSelfieCaptureClosed })
)}
</p>
</div>
<button type="button" onClick={onSelfieCaptureClosed} className="usa-sr-only">
{t('doc_auth.buttons.close')}
</button>
{!loadedAcuantCamera && (
<button type="button" onClick={onSelfieCaptureClosed} className="usa-sr-only">
{t('doc_auth.buttons.close')}
</button>
)}
</>
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,47 @@ it('shows the Acuant div when the script has loaded', () => {
expect(container.querySelector('#acuant-face-capture-container')).to.exist();
expect(container.querySelector('.acuant-capture-canvas__spinner')).not.to.exist();
});

it('shows the fullscreen close button before acuant is hydrated in', () => {
// Render the AcuantSelfieCaptureCanvas component with some text
const imageCaptureText = 'Face not found';
const { rerender, container, queryByRole } = render(
<DeviceContext.Provider value={{ isMobile: true }}>
<AcuantContext.Provider value={{ isReady: true }}>
<AcuantSelfieCaptureCanvas imageCaptureText={imageCaptureText} />
</AcuantContext.Provider>
</DeviceContext.Provider>,
);
// Check that the button exists
expect(queryByRole('button')).to.exist();

// Mock how Acuant sets up the dom by creating this structure of divs
// '#acuant-face-capture-container>#acuant-face-capture-camera>#cameraContainer'
const acuantFaceCaptureDiv = document.createElement('div');
acuantFaceCaptureDiv.id = 'acuant-face-capture-camera';
const acuantFaceCaptureContainer = container.querySelector('#acuant-face-capture-container');
acuantFaceCaptureContainer.appendChild(acuantFaceCaptureDiv);
expect(
container.querySelector('#acuant-face-capture-container>#acuant-face-capture-camera'),
).to.exist();

// Mock how Acuant sets up the shadow dom with the #cameraContainer div inside it
const cameraContainer = document.createElement('div');
cameraContainer.id = 'cameraContainer';
const shadow = container
.querySelector('#acuant-face-capture-camera')
.attachShadow({ mode: 'open' });
shadow.appendChild(cameraContainer);

// Rerender the component, the shadow dom continues to exist
const newImageCaptureText = 'Too many faces';
rerender(
<DeviceContext.Provider value={{ isMobile: true }}>
<AcuantContext.Provider value={{ isReady: true }}>
<AcuantSelfieCaptureCanvas imageCaptureText={newImageCaptureText} />
</AcuantContext.Provider>
</DeviceContext.Provider>,
);
// The button now disappears
expect(queryByRole('button')).not.to.exist();
});