Skip to content

Add SubmissionPending React component#3965

Merged
aduth merged 3 commits intomasterfrom
aduth-submission-pending-component
Jul 23, 2020
Merged

Add SubmissionPending React component#3965
aduth merged 3 commits intomasterfrom
aduth-submission-pending-component

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jul 22, 2020

Relates to: LG-3023
Complements: #3964

Changes:

  1. Adds a new React component to display interstitial that upload is pending.
    • Why: Once the user has submitted their documents for upload, it will take some time for the server to respond with the result of the upload. An interstitial screen should be shown to explain to the user that there is a brief delay, and that the page will update automatically once complete.
    • Note: This is working from the current design iteration. It's expected this could change. Of note, the decorative graphic image should be replaced.
  2. Upgrades Sinon dependency to latest version.
    • Why: The runAll method of Sinon fake timers was added in a later release. These is necessary for fake timer utilities used in verifying behavior of the SubmissionPending component.
    • Note: The timer code is likely not going to be necessary in a final implementation, which would instead rely on events associated with the completion of the network request. The upgrade and demonstration of a timer test lifecycle hook could prove useful for future effort, however.

Screen recording:

submission-pending mov

Comment on lines 10 to 8
Copy link
Contributor Author

@aduth aduth Jul 22, 2020

Choose a reason for hiding this comment

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

Upgrading Sinon had me encounter an issue with how the focus-trap library was stubbed to assume the usage of a constructor. I also identified this in #3938, but the previous usage here does not appear to be correct. The default export of focus-trap is intended to be called as a factory function, not as a constructor.

See documentation for the version of the package used in the project:

https://github.com/davidtheclark/focus-trap/tree/2.3.0#focustrap--createfocustrapelement-createoptions

Copy link
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

aduth added 3 commits July 23, 2020 16:39
**Why**: The `runAll` method of Sinon fake timers was added in a later release. These will be necessary for fake timer utilities added in a subsequent commit.
**Why**: Once the user has submitted their documents for upload, it will take some time for the server to respond with the result of the upload. An interstitial screen should be shown to explain to the user that there is a brief delay, and that the page will update automatically once complete.
**Why** Since the rendering of this component is now controlled by React Suspense, it does not need to manage its own removal.
@aduth aduth force-pushed the aduth-submission-pending-component branch from 57452ed to befe830 Compare July 23, 2020 20:48
@aduth
Copy link
Contributor Author

aduth commented Jul 23, 2020

Complements: #3964

The merging of #3964 had a pretty dramatic effect here. I've finished rebasing the branch, where SubmissionPending is now rendered as the fallback of the Suspense component introduced with #3964. This was always intended, and the user-facing flow is starting to take a bit more shape:

sending-interstitial mov

But as part of this, the component's onComplete prop is no longer needed, nor the useEffect with timers to call it. With no timers, the test utility for useFakeTimers was not needed either, and was removed.

But since this could be useful for future work, I made a point to deliberately keep the commit history in a shape where it could be easy to restore this code if needed.

See: 59a2fde, befe830

@aduth aduth merged commit f0dc720 into master Jul 23, 2020
@aduth aduth deleted the aduth-submission-pending-component branch July 23, 2020 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants