feat(elements): Consider ValidityState in FieldState #3594
Conversation
🦋 Changeset detectedLatest commit: 76391bc The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…dation-not-considered-in-field
| * @see https://developer.mozilla.org/en-US/docs/Web/API/ValidityState | ||
| */ | ||
| const enrichFieldState = (validity: ValidityState | undefined, fieldState: FieldStates) => { | ||
| return validity?.valid === false ? FIELD_STATES.error : fieldState; |
There was a problem hiding this comment.
valid
A boolean value that is true if the element meets all its validation constraints, and is therefore considered to be valid, or false if it fails any constraint.
There was a problem hiding this comment.
I also thought about doing this instead:
return fieldState === FIELD_STATES.idle ? validity?.valid : fieldStateI guess it's a question of what should take precedence. The validity or our own fieldState
| @@ -149,7 +166,7 @@ const useForm = ({ flowActor }: { flowActor?: BaseActorRef<{ type: 'SUBMIT' }> } | |||
|
|
|||
| return { | |||
| props: { | |||
| [`data-${validity}`]: true, | |||
| ...(errors.length > 0 ? { 'data-global-error': true } : {}), | |||
There was a problem hiding this comment.
It didn't make much sense to me to have data-valid/data-invalid on the <form> if that's already placed further below in the DOM tree. And in those instances we refer to the <FieldError> state, not <GlobalError>.
So renaming this to make it more clear that this is about global errors.
|
|
||
| return { | ||
| hasValue, | ||
| props: { | ||
| [`data-${validity}`]: true, |
There was a problem hiding this comment.
Radix already adds data-valid/data-invalid correctly to the field, so this is removed to not end up in a situation where both data-valid/data-invalid can exist at the same time.
| <RadixValidityState> | ||
| {validity => { | ||
| const enrichedFieldState = enrichFieldState(validity, fieldState); | ||
|
|
||
| return ( | ||
| <ValidityStateContext.Provider value={validity}> | ||
| {typeof children === 'function' ? children(enrichedFieldState) : children} | ||
| </ValidityStateContext.Provider> | ||
| ); | ||
| }} | ||
| </RadixValidityState> |
There was a problem hiding this comment.
Main gist of this PR. Using Context here to not have to pass around values through props 👍
…dation-not-considered-in-field
LekoArts
deleted the
lekoarts/sdk-1638-bug-client-side-field-validation-not-considered-in-field
branch
Description
This PR adds the ValidityState (https://www.radix-ui.com/primitives/docs/components/form#validitystate) to our FieldState logic. So e.g. if someone enters an invalid email address into a
<input type="email" />and focus away, our<FieldState>will now report anerror. Previously it didn't do anything.In this PR also some data attributes were removed/renamed, see changeset + inline comments for more details.
Docs PR: clerk/clerk-docs#1179
Fixes SDK-1638
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change