[Task] [User Profile] useRef in Step1FormErrorHeader

[shindigira]

close #102

Changes

• Upon submission invalidation, the user's view is smooth scrolled to the Step1FormErrorHeader
• utilized useRef in Step1FormErrorHeader to focus an error element

How to Test

• In the Complete your Use Profile click the submit button
• Ensure no regressions when clicking on the error links; should focus and smooth scroll to the appropriate item.

Screenshot

[meissadia]

Suggested change

// navigate('/landing')

[shindigira]

Once the issue is solved, navigating should done this way. Using window.location.href causes a hard refresh.
I'm keeping the comment here since it will eventually be uncommented.

[meissadia]

Nice!

When there are errors upon clicking Submit, would it make sense to focus on the "There was a problem completing your profile" box that lists all of the errors? That way users get the overview of what needs to be corrected before they will be able to proceed.

[shindigira]

Nice!

When there are errors upon clicking Submit, would it make sense to focus on the "There was a problem completing your profile" box that lists all of the errors? That way users get the overview of what needs to be corrected before they will be able to proceed.

Good thinking. I will add this to this PR.

[shindigira]

@meissadia Added an additional UX of a smooth scrolling to the error heading after hitting the submit button (if there are errors).

Please re-review and focus that part of this PR.

[meissadia]

@shindigira I've pulled the latest but the app is crashing:

I don't see errorFormReference defined anywhere in the project.

[shindigira]

@shindigira I've pulled the latest but the app is crashing:

I don't see errorFormReference defined anywhere in the project.

@meissadia Removed that. Should work now! Thanks for the catch.

[billhimmelsbach]

There's some great stuff in here @shindigira!

I don't want to hold up this PR, but the "useRef" part you've added still uses document.querySelector when creating the ref, instead of a declarative approach using refs. The react-hook-form documentation has an example on their FAQ page that they recommend for scrolling an input into view that uses the declarative ref approach.

https://www.react-hook-form.com/faqs/#Howtosharerefusage

I don't want to hold up this PR, and there's more code to write with this approach, but when I think about using refs declaratively, this is the kind of approach I was imagining.

[shindigira]

There's some great stuff in here @shindigira!

I don't want to hold up this PR, but the "useRef" part you've added still uses document.querySelector when creating the ref, instead of a declarative approach using refs. The react-hook-form documentation has an example on their FAQ page that they recommend for scrolling an input into view that uses the declarative ref approach.

https://www.react-hook-form.com/faqs/#Howtosharerefusage

I don't want to hold up this PR, and there's more code to write with this approach, but when I think about using refs declaratively, this is the kind of approach I was imagining.

That approach you linked is not dynamic by having to manually declare each ref top-level. Could keep an array or map object of refs (e.g. useRef(refArray.map(() => createRef()))) in Step1Form1 but would rather keep it as is.

[billhimmelsbach]

That approach you linked is not dynamic by having to manually declare each ref top-level. Could keep an array or map object of refs (e.g. useRef(refArray.map(() => createRef()))) in Step1Form1 but would rather keep it as is.

I think I'd prefer it then if you just go back to using document.querySelector without refs since the code now is confusing to me, since you're setting something that should be declarative with an imperative query of the document body. Generally we want to avoid imperative code with react, but if you don't want to for this PR, let's just leave it one or the other.

[shindigira]

That approach you linked is not dynamic by having to manually declare each ref top-level. Could keep an array or map object of refs (e.g. useRef(refArray.map(() => createRef()))) in Step1Form1 but would rather keep it as is.

I think I'd prefer it then if you just go back to using document.querySelector without refs since the code now is confusing to me, since you're setting something that should be declarative with an imperative query of the document body. Generally we want to avoid imperative code with react, but if you don't want to for this PR, let's just leave it one or the other.

Reverted; and I agree about avoiding imperative code.