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

Allow custom focusOnError #3580

Closed
1 task done
Fronix opened this issue Apr 5, 2023 · 1 comment · Fixed by #3590
Closed
1 task done

Allow custom focusOnError #3580

Fronix opened this issue Apr 5, 2023 · 1 comment · Fixed by #3590
Labels
feature Is a feature request help wanted

Comments

@Fronix
Copy link
Contributor

Fronix commented Apr 5, 2023

Prerequisites

What theme are you using?

core

Is your feature request related to a problem? Please describe.

Our forms sometimes have disabled fields that are automatically filled by doing some type of action.
Example: Address fields that are readonly but are filled if you search and select the adress, this will then automatically fill in the fields. The adress fields are required because it wouldn't make sense to have the search field required.

The problem here is that when i use focusOnFirstError it wont trigger on fields that are disabled or readonly.

let field = this.formElement.current.elements[elementId];
if (!field) {
// if not an exact match, try finding an input starting with the element id (like radio buttons or checkboxes)
field = this.formElement.current.querySelector(`input[id^=${elementId}`);
}
if (field.length) {
// If we got a list with length > 0
field = field[0];
}
if (field) {
field.focus();
}
}

This might be a very niche issue but it's quite important from a UX standpoint.

Describe the solution you'd like

There are different solutions that would work.

  1. Let us provide our own callback for focusOnError, this would give the most freedom.
  2. Add a check if the element is disabled/readonly and instead call element.scrollIntoViewIfNeeded(true) might have to use element.scrollIntoView() since support for IfNeeded is not complete.

Describe alternatives you've considered

No response

@Fronix Fronix added feature Is a feature request needs triage Initial label given, to be assigned correct labels and assigned labels Apr 5, 2023
@heath-freenome heath-freenome added help wanted and removed needs triage Initial label given, to be assigned correct labels and assigned labels Apr 7, 2023
@heath-freenome
Copy link
Member

heath-freenome commented Apr 7, 2023

@Fronix If I had the time to do this any time soon, I would make the focusOnFirstError property also take a callback as an option. Something like:

  /** If set to true, then the first field with an error will receive the focus when the form is submitted with errors
   */
  focusOnFirstError?: boolean | (error: RJSFValidationError) => void;

And then change the code in Form to look something like:

if (focusOnFirstError) {
  if (typeof focusOnFirstError === 'function') {
    focusOnFirstError(schemaValidation.errors[0]);
  } else {
    this.focusOnError(schemaValidation.errors[0]);
  }
}

Are you willing to make this change yourself and push a PR, adding the necessary tests?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Is a feature request help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants