Skip to content

ControlWithError: Show validating state when transitioning from error state #71260

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.

Sign up for GitHub

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

Merged
mirka merged 10 commits into from
Aug 26, 2025
Merged

ControlWithError: Show validating state when transitioning from error state #71260

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
dea1939
Add tests
mirka Aug 19, 2025
1046981
ControlWithError: Show validating state when transitioning from error…
mirka Aug 19, 2025
ef64fa6
Add changelog
mirka Aug 19, 2025
125b759
Mimic a stable server response time
mirka Aug 20, 2025
0adf893
Prefer `toBeVisible` in tests
mirka Aug 20, 2025
a50671f
Fix useEffect return type weirdness
mirka Aug 20, 2025
ffa9e22
Add test for deferring local validity state until server response is …
mirka Aug 22, 2025
b256311
Add play function
mirka Aug 22, 2025
8f4198b
Wait for async response before showing local state
mirka Aug 22, 2025
bc50407
Don't export test story
mirka Aug 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
ControlWithError: Show validating state when transitioning from error… 
… state
  • Loading branch information
@mirka
mirka committed Aug 19, 2025
commit 1046981d3ce1ca62afb06656d8945e6b76b2fc25
3 changes: 3 additions & 0 deletions packages/components/src/validated-form-controls/control-with-error.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ function UnforwardedControlWithError< C extends React.ReactElement >(
case 'validating': {
// Wait before showing a validating state.
const timer = setTimeout( () => {
validityTarget?.setCustomValidity( '' );
setErrorMessage( validityTarget?.validationMessage );
Copy link
Copy Markdown
Member

@jsnajdr jsnajdr Aug 20, 2025

Choose a reason for hiding this comment

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

Maybe in this case we want to setErrorMessage( undefined )? There is no error while validation is running.

The use case is interaction of the onValidate custom (server-side) validation with client-side validation, e.g., when there is something like <input type="email"> and the value is not email.

In that case, when additional custom validation is in flight, and status is validating, do we want errorMessage to show the "not email" error or not.

I'm not sure if we even support this combination of custom and native validation.

Copy link
Copy Markdown
Member Author

@mirka mirka Aug 20, 2025

Choose a reason for hiding this comment

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

I'll be adding tests soon to help document these intended behaviors, but yes, we do support the combination of attribute-based and customValidity. I tried to match how the native APIs handle it, in that the customValidity message will be prioritized when it's a non-empty string, but otherwise the rest of the attribute-based validation errors will remain. It also matches the native browser logic of how errors messages are displayed — there can be multiple errors but only one error message is shown at a time.

In that case, when additional custom validation is in flight, and status is validating, do we want errorMessage to show the "not email" error or not.

That's a good question. I'm not sure if there's a universally better behavior, but I think in the general case, yes, we do want to show the "not email" error without waiting for the custom validator result.

Let's say there's a 50/50 chance of the custom async validator responding as error, and we wait for the async result before notifying the user about the "not email" error. This means that 50% of the time we made the user wait for no reason, because we already knew about the "not email" error.

Copy link
Copy Markdown
Member Author

@mirka mirka Aug 22, 2025

Choose a reason for hiding this comment

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

Turns out, in practice it did feel better to make the user wait in all cases 😅 Going with setErrorMessage( undefined ) as in your suggestion.


setStatusMessage( {
type: 'validating',
message: customValidity.message,
Expand Down
Loading