Persisted form state with sessionStorage

[benjaminleonard]

Whilst taking a look at useBlocker / usePrompt — I thought that in most cases it's probably preferable to instead hang on to the form state after clicking off and returning to the form rather than blocking navigation. GitHub does this with issues and it's really useful.

It's pretty simple to do. We get the values form the form, turn them into a JSON object, add to sessionStorage using the forms ID as a key. Then whenever we go back to the form we look up with that same key, check that it's not older than 5 minutes (bit of an arbitrary value I picked) and retrieve it, looping over the values and setting them on the form. When a form is submitted the state is cleared.

Some limitations —

1. It doesn't work with the listboxes that use an API to retrieve their items. This is because the value doesn't yet exist to be selected.
2. It doesn't work with file inputs
3. Validation doesn't seem to be applied when the form state is retrieved
4. Doesn't currently handle nested values like blockSize but I don't think this is too complicated to support
5. Currently just implemented on the SideModalForm

[david-crespo]

Cool idea. We have to be careful because this is where the console starts to cross the line into being an application rather than a client to the real application, which is Nexus. To me that means trying to make it as clear as possible what the logic around peristence is. My main concern is about persisting too much or too often. I like the idea behind the 5 minute timeout, but I could see someone coming to expect it to be longer, maybe arbitrary length, just because they’ve never gone longer than 5 minutes, and then being pretty upset when they left it longer and something they wrote out disappeared. I wonder if we should have a clear form button. With RHF that should be very easy, just call reset() on click.

[benjaminleonard]

Good points — I feel like it's worthwhile persisting for smaller forms (namely just the side modal ones) and not the full page forms. The stakes are pretty low for those, that way there's a convenience but it's not significant if you were to lose it.

We could also drop the timeout and just let the expiration of sessionStorage naturally remove them. I don't see an expiration on GitHubs sessionStorage items when you go back to creating a new issue. Happy to do that if you think it's confusing (since there's no good way of documenting that behaviour).

Full page forms i'm less sold on, I think a useBlocker could catch most of the accidental navigating away. It doesn't account for a situation where a user has forgotten to add an SSH key and needs to reload the form after adding it, but that's niche enough that I'm not too worried about it.

[just-be-dev]

Yeah, this is neat. Something I think could be beneficial is to be very explicit about what we're doing and why. In this case (closing a halfway completed side modal) we could pop open a toast saying something along the lines of "we've temporarily saved your work" with a link to the docs and perhaps and option to clear. I'm not sure if that would be too much, the point being here is that we're just really explicit about what's happening and why. We've put a lot of effort into other areas of the product to give that same sort of clarity.

[benjaminleonard]

Let me give that some thought — also took a stab at #1523 just because, but happy to leave that and get this implemented on full page forms also.

Toasts do feel too intrusive for this use case, so I'll consider if there's something else that might work. Likewise for a form reset. There might be a pattern that asks you if you want to continue where you left off when you open the form.

[david-crespo]

We could also drop the timeout and just let the expiration of sessionStorage naturally remove them.

Yeah, I think no expiration and using sessionStorage’s own scoping/expiration logic is probably the most legible/predictable option.

[just-be-dev]

I'm still thinking on this. I think the thing I really want to see from whatever solution we come up with is for the user to have explicit acknowledgement of whatever is happening. As much as possible I want to reduce unexplained, automatic behavior. I really don't want user's to be asking themselves "wtf just happened?" Granted, I know for forms auto filling that's less of a big deal, but still.

There might be a pattern that asks you if you want to continue where you left off when you open the form.

I think this would be even better. A mechanism to opt-in to restore a session instead of it automatically happening. The only challenge here is the user may not remember what was automatically persisted from the last session. We could allow them to fill it in and give them an option to clear it. I'm not sure. Worth some extra thought though.

I'm not against this being merged as is necessarily nor do I want to overburden the UI. We could always land this and iterate on how it feels to us and the team.

[benjaminleonard]

Couple of options for restoring progress, opt in and opt out. Copy needs a bit of work.

Opt in

Opt out

[benjaminleonard]

Any thoughts on either of these approaches? @david-crespo @zephraph

[just-be-dev]

Given the two, I think I'd got for opt-out. In that case you're able to see what's there before making a decision. In the opt-in case, you don't know what values you're opting into.

[just-be-dev]

Another potential consideration: we're likely going to want to announce to a screen reader what's happening in each case. For a sighted user it's easy to scan the form quickly.

Well, thinking more about that, I think it would be the same UX either way, so it's no big deal.

That doesn't need to implemented in this PR, but it'd be good to have an issue to track it.

[benjaminleonard]

Great feedback! I've implemented both of those. Last thing on my list is updating @oxide/design-system with info tokens (using the blue).

[benjaminleonard]

Ready to go now

[just-be-dev]

This looks good to me. It wouldn't hurt for us to follow up with some E2E tests just to exercise a few different scenarios to ensure we don't accidentally break it in the future. Otherwise, great job @benjaminleonard!

[david-crespo]

I think this is confusing copy. "Session" gives it too much weight as a concept, and it makes it sound like a unified thing rather than bits of state per-form. I think something really concrete and flat like "Restored form state" would be better. There's still the question "from where?" when you see that — maybe the icon should be a hover tooltip that says something to the effect of "We will keep the contents of an incomplete form around until you clear it or close the tab or window"

[benjaminleonard]

Made the copy change, let me think about the tooltip, since it'd require breaking the message component out for this use one case.

[david-crespo]

We talked about this a little in chat, but I want to put this on hold. There are a few issues that might not be that bad to fix, but they do add up, and since this to me is a nice-to-have, I don't think it's worth the time right now, and more importantly, it puts a burden on us in perpetuity to make sure forms remain compatible with the persistence mechanism. Failure to do so can manifest in weird ways that are hard to notice.

Why only nice-to-have

You compare this to text field restoration on GitHub issues, but most of the time, I don't think filling out a form is much like writing out a comment or PR description. Writing can have an arbitrary amount of texture and detail to it, and when you lose something you've written that can be very frustrating. I could certainly be convinced otherwise, but I doubt the same is true of most of our forms, which are 4-8 fields of structured data. We have few big text fields and I don't see people sitting there thoughtfully composing their disk description with reference to a bunch of other browser tabs.

Alternate approaches

I experimented with using defaultValues at form initialization time instead of calling setValue on every field, and I think that's probably a better approach overall. We would just have to trigger a validation on everything after restoring. One problem is that once you use a not-really-default values object for defaultValues, calling reset() with no arguments will only reset you to those defaults, which is probably never what we want. The fix for that is to pass the true defaultValues to reset() whenever you call it. That's not a big deal.

Issues I found

• Form is not saved on back button because we do the persisting in the dismiss callback
  • Would have to use one of the RR blocker hooks instead/also
• Banner should be cleared once restored form state is edited (just my opinion, but at that point the data no longer represents what was restored and should be considered its own thing)
• Misc. potential de/serialization issues
  • I think this is only a problem for us if we try to use a non-serializable value as a form field value. The only plausible one we would use is Date (we wouldn't likely try to set a regex or a function as a field value) and while we are currently not doing that anywhere, we could. That might not be a big deal — we could build in special handling, but it's something we have to keep an eye on
  • An easy way around this would be to use superjson for ser/de, which is designed precisely for this
• Disk source radio bug on disk create (see gif below)
  • RadioGroup is not updated even though the form state actually does have the right value
  • blockSize sticks around in the values object for non-blank sources even though it's not relevant (I think can be fixed by only calling setValue on the top-level keys instead of recursing)