upcoming: [M3-8092] - Linode Create Refactor - Scroll Errors Into View

[bnussman-akamai]

Description 📝

• Makes errors that are notices scroll into view on the Linode Create flow ⛔️

Preview 📷

Screen.Recording.2024-05-09.at.4.53.39.PM.mov

How to test 🧪

Prerequisites

• Turn on Linode Create v2 flag using local dev tools 🏴

Verification steps

• Verify all errors scroll into view ↕️
• Test error states in general 👀

As an Author I have considered 🤔

• 👀 Doing a self review
• ❔ Our contribution guidelines
• 🤏 Splitting feature into small PRs
• ➕ Adding a changeset
• 🧪 Providing/Improving test coverage
• 🔐 Removing all sensitive information from the code and PR description
• 🚩 Using a feature flag to protect the release
• 👣 Providing comprehensive reproduction steps
• 📑 Providing or updating our documentation
• 🕛 Scheduling a pair reviewing session
• 📱 Providing mobile support
• ♿ Providing accessibility support

[abailly-akamai]

The error handling works well!

I know you're probably trying to keep the scope small and handle error handling for the linode create v2 only - can't help but wonder how this will scale when trying to implement more forms with react-hook-forms. It's a lot of ref business which can get a little overwhelming from a developer persepctive.

Not that i have a better solution to suggest, just wanted to raise the thought. I was also working with refs on a POC for the formik error handling (it's far more constraining since it's already implemented everywhere those are field errors which react-hook-forms already handles well), but perhaps there's something to explore here too about parsing a larger ref (aka a form container) for errors. Again, not saying it's better than what you propose here, just trying to add water to the well

[github-actions]

Coverage Report: ✅
Base Coverage: 81.55%
Current Coverage: 81.55%

[dwiley-akamai]

When the password complexity doesn't meet the minimum requirement, the error doesn't get scrolled into view:

Video

Screen.Recording.2024-05-15.at.11.19.45.AM.mov

[bnussman-akamai]

I re-requested reviews because scrolling errors into view works as expected using a new temporary solution. When we decide on a new util, we can adopt it here later. Just trying to keep this effort moving

[dwiley-akamai]

The scrolling of errors into view looked good across the board from my testing ✅
Code review ✅

[abailly-akamai]

@bnussman-akamai so here's the gist!

In react-hook-form, you can use the handleSubmit function to perform validation and then scroll to the error. The handleSubmit function takes two arguments: the first is the function to execute when the form is valid, and the second is the function to execute when the form is invalid.

I pushed a small fix to do it that way - please test and lemme know if we're at parity otherwise I'll revert it.

https://linode.github.io/manager/development-guide/05-fetching-data.html#scrolling-to-errors

[abailly-akamai]

Looks good and works as expected! (granted i contributed, maybe another review could be useful)

[abailly-akamai]

I don't think we're at a place for an abstraction to load a default config, but perhaps something to consider as the plugin usage grow. Cause so far as i can understand, to achieve consistency in our forms we already have

mode: 'onBlur',
shouldFocusError: false,

as default values.

If not an abstraction, maybe an importable config down the line?

[bnussman-akamai]

Yeah, I think an improbable config is a good medium!