LG-6314: Add Start Over and Cancel links to password confirmation#6350
LG-6314: Add Start Over and Cancel links to password confirmation#6350
Conversation
**Why**: For feature parity with the existing screen, a user should have the option to start over or cancel the proofing process. changelog: Upcoming Features, Identity Verification, Add password confirmation step
| <input type="hidden" name="_method" value={method} /> | ||
| <input ref={csrfRef} type="hidden" name="authenticity_token" /> | ||
| </form>, | ||
| document.body, |
There was a problem hiding this comment.
do we need some sort of key to reference these at the document body? so that two ButtonTo's on the same page don't overwrite each other's forms?
There was a problem hiding this comment.
do we need some sort of key to reference these at the document body? so that two ButtonTo's on the same page don't overwrite each other's forms?
If there were two ButtonTo, each would have and submit their own form, so I don't think they'd conflict with each other.
The markup would look like...
<body>
<div id="app-root">
<button></button>
<button></button>
</div>
<form></form>
<form></form>
</body>| startOverURL: /** @type {string} */ (''), | ||
| cancelURL: /** @type {string} */ (''), |
There was a problem hiding this comment.
awesome that we can clean up the existing flow by using this too
| import Button from './button'; | ||
| import type { ButtonProps } from './button'; | ||
|
|
||
| interface ButtonToProps extends ButtonProps { |
There was a problem hiding this comment.
would we ever need to add params here that could be posted? or is that the kind of thing we'd add if/when we need it?
There was a problem hiding this comment.
would we ever need to add params here that could be posted? or is that the kind of thing we'd add if/when we need it?
Possibly? I liken this to Rails' button_to, which (TIL) supports a params hash for just that purpose. But I heavily subscribe to the school of YAGNI, so I'm not in any hurry to add it.
| let searchParams: URLSearchParams; | ||
|
|
||
| try { | ||
| parsedURLOrParams = new URL(urlOrParams); |
There was a problem hiding this comment.
is there a reason we moved away from the URL class? I'd love to be able to rely on something like that instead of having to build our own
There was a problem hiding this comment.
is there a reason we moved away from the
URLclass? I'd love to be able to rely on something like that instead of having to build our own
I went through a few iterations of this. The main problem with the URL class is that this function is meant to be quite flexible with its input values (absolute URLs, paths, querystring-only), and it's hard to map each of those to a corresponding return value of the same form using URL.
That being said, I couldn't actually find any existing usage of the querystring parameter input, so we could adjust this to assume that both the input and output would be a full URL.
There was a problem hiding this comment.
Also, to be more specific about why it was changed here: We had been initializing startOverURL and cancelURL with paths. A URL constructor won't parse a path fragment on its own (new URL('/foo') throws). We could adjust this to parse relative to the current URL (e.g. new URL('/foo', window.location.href)), but at that point is when it becomes harder to determine the logic around how to return the expected value (parsedURL.href ? parsedURL.pathname + parsedURL.search ? How do we determine which to use?).
There was a problem hiding this comment.
That being said, I couldn't actually find any existing usage of the querystring parameter input, so we could adjust this to assume that both the input and output would be a full URL.
Simplifying supports simplifies the implementation! 0f3f511
To simplify specs, avoid needing to specify them
Simplify supported use-cases to only those we care about, so that we can simplify the implementation #6350 (comment)
Why: For feature parity with the existing screen, a user should have the option to start over or cancel the proofing process.
Testing Instructions:
You will need to set the step as enabled in your local
config/application.yml:?step=password_confirmin the URLImplementation Notes:
This includes quite a bit of shuffling of components already implemented in document capture, so that they can be reused in both places.
This also implements a new "flow context", whose purpose overlaps with the one being introduced by @nprimak in #6323.
Screenshots: