-
Notifications
You must be signed in to change notification settings - Fork 120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LG-3787: Create waiting experience for address checking, ID resolution and DocAuth #4517
Conversation
<%= button_to( | ||
t('forms.buttons.continue'), | ||
url_for, | ||
method: :put, | ||
form: { | ||
class: 'button_to read-after-submit', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: Research why accessible-forms.js exists, since in cases like this it prevents screen readers from announcing intended status texts after a submission. At the very least, it could be improved to respect event.defaultPrevented
in a submit
event.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to check my understanding: this PR submits the form on the page as a .fetch
request, then checks the HTML response we get back to see if it should advance or not?
Should we consider making a separate API endpoint for polling like we did for the doc auth endpoints?
Correct. It essentially submits the same form in JavaScript, then requests the The nice thing about it is that it doesn't require any changes to the code that already exists, which we may still want as the progressive enhancement fallback for environments with JavaScript disabled (as long as we care to support no-JS in this flow, that is). To your question: We could, yes. Per the above, it adds more code that we don't necessarily need. On the other hand, it is a bit unconventional and wasteful to use |
One thought here is to have an endpoint which simply returns the active step of the current user. This would make the logic a bit easier to know whether the user has completed the current step to redirect them along. It could possibly even absorb what we have for One complication may be that the |
Added to the CAC verification flow in 9ba5029. From the ticket description, it's not clear to me where else we have these sorts of async flows. I don't see any other references in code to this @zachmargolis, @bpdesigns Can you clarify if there are other places this spinner should be implemented? |
On second though, I don't know that we need anything new or custom here? |
**Why**: Act more like real button, allow natural grow (or overridden width) on container
**Why**: Initial response could redirect immediately. Also consolidates error handling consistently.
**Why**: Not concerned with the response body
**Why**: `npm test` won't capture files unless suffixed with "spec"
cec86b0
to
f7260c8
Compare
**Why**: Kinda the motivation behind the discussion here: #4517 (comment)
**Why**: This is out of sync with current guidance, where buttons which trigger a waiting experience have since been refined within the IdP to display with a pulsing dot animation. See: LG-3730, LG-3787, 18F/identity-idp#4517
* Remove Spinner component button variant **Why**: This is out of sync with current guidance, where buttons which trigger a waiting experience have since been refined within the IdP to display with a pulsing dot animation. See: LG-3730, LG-3787, 18F/identity-idp#4517 * Merge 3.0.1 into unreleased changelog A 3.0.1 hasn't yet been published
Why: As a user, I want to know that my data is processing so that I understand that things are progressing as expected.
Screenshots:
Animation:
Message after long wait:
To do:
Implement in other async steps(Edit: Done in 9ba5029)Add spec for(Edit: Done in 0d4d3da)spinner-button.html.erb
partial, which has increasingly non-trivial logicAdd spec for(Edit: Done in 96b774b)form-steps-wait.js