Skip to content
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

Fixes issue #3059 - scroll to first validation error on BaseForm #3060

Merged
merged 2 commits into from
Nov 17, 2023

Conversation

alexcottner
Copy link
Contributor

Fixes issue 3059. When a user receives a validation error for a specific property, it will now scroll and center the label for that property in their view.

If an error is thrown for the form rather than a specific property, it will scroll to the top of the form where the full errors list appears.

simplescreenrecorder-2023-11-15_13.41.51.mp4

<FormWithTheme
{...restProps}
focusOnFirstError={errorFocus}
error
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 what's error here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have no idea why that was there, removed.

Comment on lines +31 to +32
formRef.current
.querySelector(`[for="root${err.property.replace(/\./g, "_")}"]`)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
formRef.current
.querySelector(`[for="root${err.property.replace(/\./g, "_")}"]`)
const fieldName = err.property.replaceAll(".", "_");
formRef.current
.querySelector(`[for="root${fieldName}"]`)

This could be fragile since it seems to rely on an internal RSJF naming. Maybe we could add a link to it, I can't find it in upstream source https://github.com/rjsf-team/react-jsonschema-form/blob/main/packages/bootstrap-4/src/FieldErrorTemplate/FieldErrorTemplate.tsx

return errors;
}}
/>
);
Copy link
Contributor

@leplatrem leplatrem Nov 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we add a test here to make sure the query selector that we use in the code is found in the page? .querySelector(`[for="root${err.property.replace(/\./g, "_")}"]`)
This way if RJSF changes the way it names its fields, the test would break

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, added

@alexcottner alexcottner merged commit fa4832e into Kinto:master Nov 17, 2023
6 checks passed
@alexcottner alexcottner deleted the validation-error-move branch November 17, 2023 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants