From e186746f231d1cd37e33120785935e5243283c7d Mon Sep 17 00:00:00 2001 From: George Desipris Date: Mon, 25 Sep 2023 18:20:38 +0300 Subject: [PATCH 1/3] feat(clerk-js): Improve password error feedback feat(clerk-js): Add PasswordInput validation debouncing refactor(clerk-js): Remove unused props fix(clerk-js): Add infoText for VerifyDomainPage fix(clerk-js): Smooth error animation and min height --- .../OrganizationProfile/InviteMembersForm.tsx | 9 +- .../ProfileSettingsPage.tsx | 2 +- .../OrganizationProfile/VerifyDomainPage.tsx | 2 +- .../ui/components/SignIn/ResetPassword.tsx | 5 +- .../src/ui/components/SignUp/SignUpStart.tsx | 4 +- .../components/UserProfile/PasswordPage.tsx | 6 +- .../clerk-js/src/ui/elements/CodeControl.tsx | 19 +- .../clerk-js/src/ui/elements/FormControl.tsx | 227 +++++++++--------- .../src/ui/elements/PasswordInput.tsx | 46 +++- packages/clerk-js/src/ui/hooks/usePassword.ts | 33 +-- .../clerk-js/src/ui/hooks/useSetTimeout.ts | 2 +- .../src/ui/primitives/FormControl.tsx | 5 +- .../ui/primitives/hooks/useFormControl.tsx | 24 +- .../src/ui/styledSystem/animations.ts | 15 +- .../clerk-js/src/ui/utils/useFormControl.ts | 169 ++++--------- .../clerk-js/src/utils/passwords/password.ts | 5 +- 16 files changed, 272 insertions(+), 301 deletions(-) diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/InviteMembersForm.tsx b/packages/clerk-js/src/ui/components/OrganizationProfile/InviteMembersForm.tsx index c8924e71202..5f8fa230468 100644 --- a/packages/clerk-js/src/ui/components/OrganizationProfile/InviteMembersForm.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/InviteMembersForm.tsx @@ -53,19 +53,12 @@ export const InviteMembersForm = (props: InviteMembersFormProps) => { const { props: { /* eslint-disable @typescript-eslint/no-unused-vars */ - enableErrorAfterBlur, - errorText, - hasLostFocus, - isFocused, setError, setWarning, - setSuccessful, - successfulText, - warningText, + setSuccess, validatePassword, setHasPassedComplexity, hasPassedComplexity, - /* eslint-enable @typescript-eslint/no-unused-vars */ ...restEmailAddressProps }, } = emailAddressField; diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/ProfileSettingsPage.tsx b/packages/clerk-js/src/ui/components/OrganizationProfile/ProfileSettingsPage.tsx index 2002c34ff6b..0c99fdc90a1 100644 --- a/packages/clerk-js/src/ui/components/OrganizationProfile/ProfileSettingsPage.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/ProfileSettingsPage.tsx @@ -35,7 +35,7 @@ export const ProfileSettingsPage = withCardStateProvider(() => { } const dataChanged = organization.name !== nameField.value || organization.slug !== slugField.value; - const canSubmit = (dataChanged || avatarChanged) && !slugField.errorText; + const canSubmit = (dataChanged || avatarChanged) && slugField.feedbackType !== 'error'; // eslint-disable-next-line @typescript-eslint/require-await const onSubmit = async (e: React.FormEvent) => { diff --git a/packages/clerk-js/src/ui/components/OrganizationProfile/VerifyDomainPage.tsx b/packages/clerk-js/src/ui/components/OrganizationProfile/VerifyDomainPage.tsx index 583b3bc1c9d..5429b09f532 100644 --- a/packages/clerk-js/src/ui/components/OrganizationProfile/VerifyDomainPage.tsx +++ b/packages/clerk-js/src/ui/components/OrganizationProfile/VerifyDomainPage.tsx @@ -48,7 +48,7 @@ export const VerifyDomainPage = withCardStateProvider(() => { type: 'text', label: localizationKeys('formFieldLabel__organizationDomainEmailAddress'), placeholder: localizationKeys('formFieldInputPlaceholder__organizationDomainEmailAddress'), - informationText: localizationKeys('formFieldLabel__organizationDomainEmailAddressDescription'), + infoText: localizationKeys('formFieldLabel__organizationDomainEmailAddressDescription'), }); const affiliationEmailAddressRef = useRef(); diff --git a/packages/clerk-js/src/ui/components/SignIn/ResetPassword.tsx b/packages/clerk-js/src/ui/components/SignIn/ResetPassword.tsx index 03b98e0b45e..a9f413d4bd9 100644 --- a/packages/clerk-js/src/ui/components/SignIn/ResetPassword.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/ResetPassword.tsx @@ -5,7 +5,7 @@ import { withRedirectToHomeSingleSessionGuard } from '../../common'; import { useCoreSignIn, useEnvironment } from '../../contexts'; import { Col, descriptors, localizationKeys, useLocalizations } from '../../customizables'; import { Card, CardAlert, Form, Header, useCardState, withCardStateProvider } from '../../elements'; -import { useConfirmPassword, usePasswordComplexity } from '../../hooks'; +import { useConfirmPassword } from '../../hooks'; import { useSupportEmail } from '../../hooks/useSupportEmail'; import { useRouter } from '../../router'; import { createPasswordError, handleError, useFormControl } from '../../utils'; @@ -18,7 +18,6 @@ export const _ResetPassword = () => { const { userSettings: { passwordSettings }, } = useEnvironment(); - const { failedValidationsText } = usePasswordComplexity(passwordSettings); const { t, locale } = useLocalizations(); @@ -38,7 +37,6 @@ export const _ResetPassword = () => { label: localizationKeys('formFieldLabel__newPassword'), isRequired: true, validatePassword: true, - informationText: failedValidationsText, buildErrorMessage: errors => createPasswordError(errors, { t, locale, passwordSettings }), }); @@ -46,7 +44,6 @@ export const _ResetPassword = () => { type: 'password', label: localizationKeys('formFieldLabel__confirmPassword'), isRequired: true, - enableErrorAfterBlur: true, }); const sessionsField = useFormControl('signOutOfOtherSessions', '', { diff --git a/packages/clerk-js/src/ui/components/SignUp/SignUpStart.tsx b/packages/clerk-js/src/ui/components/SignUp/SignUpStart.tsx index 526e03815be..08c7ed4d7b0 100644 --- a/packages/clerk-js/src/ui/components/SignUp/SignUpStart.tsx +++ b/packages/clerk-js/src/ui/components/SignUp/SignUpStart.tsx @@ -15,7 +15,7 @@ import { withCardStateProvider, } from '../../elements'; import { useCardState } from '../../elements/contexts'; -import { useLoadingStatus, usePasswordComplexity } from '../../hooks'; +import { useLoadingStatus } from '../../hooks'; import { useRouter } from '../../router'; import type { FormControlState } from '../../utils'; import { createPasswordError } from '../../utils'; @@ -49,7 +49,6 @@ function _SignUpStart(): JSX.Element { const { userSettings: { passwordSettings }, } = useEnvironment(); - const { failedValidationsText } = usePasswordComplexity(passwordSettings); const formState = { firstName: useFormControl('firstName', signUp.firstName || initialValues.firstName || '', { @@ -81,7 +80,6 @@ function _SignUpStart(): JSX.Element { type: 'password', label: localizationKeys('formFieldLabel__password'), placeholder: localizationKeys('formFieldInputPlaceholder__password'), - informationText: failedValidationsText, validatePassword: true, buildErrorMessage: errors => createPasswordError(errors, { t, locale, passwordSettings }), }), diff --git a/packages/clerk-js/src/ui/components/UserProfile/PasswordPage.tsx b/packages/clerk-js/src/ui/components/UserProfile/PasswordPage.tsx index 7a3e18e3a4a..c604d8564ed 100644 --- a/packages/clerk-js/src/ui/components/UserProfile/PasswordPage.tsx +++ b/packages/clerk-js/src/ui/components/UserProfile/PasswordPage.tsx @@ -13,7 +13,7 @@ import { useCardState, withCardStateProvider, } from '../../elements'; -import { useConfirmPassword, useNavigateToFlowStart, usePasswordComplexity } from '../../hooks'; +import { useConfirmPassword, useNavigateToFlowStart } from '../../hooks'; import { createPasswordError, handleError, useFormControl } from '../../utils'; import { UserProfileBreadcrumbs } from './UserProfileNavbar'; @@ -59,15 +59,12 @@ export const PasswordPage = withCardStateProvider(() => { const { userSettings: { passwordSettings }, } = useEnvironment(); - const { failedValidationsText } = usePasswordComplexity(passwordSettings); const passwordField = useFormControl('newPassword', '', { type: 'password', label: localizationKeys('formFieldLabel__newPassword'), isRequired: true, - enableErrorAfterBlur: true, validatePassword: true, - informationText: failedValidationsText, buildErrorMessage: errors => createPasswordError(errors, { t, locale, passwordSettings }), }); @@ -75,7 +72,6 @@ export const PasswordPage = withCardStateProvider(() => { type: 'password', label: localizationKeys('formFieldLabel__confirmPassword'), isRequired: true, - enableErrorAfterBlur: true, }); const sessionsField = useFormControl('signOutOfOtherSessions', '', { diff --git a/packages/clerk-js/src/ui/elements/CodeControl.tsx b/packages/clerk-js/src/ui/elements/CodeControl.tsx index 23588fe997c..8f82ebb7cb7 100644 --- a/packages/clerk-js/src/ui/elements/CodeControl.tsx +++ b/packages/clerk-js/src/ui/elements/CodeControl.tsx @@ -19,7 +19,7 @@ export const useCodeControl = (formControl: FormControlState, options?: UseCodeI const otpControlRef = React.useRef(); const userOnCodeEnteredCallback = React.useRef(); const defaultValue = formControl.value; - const { errorText, onChange } = formControl; + const { feedback, feedbackType, onChange } = formControl; const { length = 6 } = options || {}; const [values, setValues] = React.useState(() => defaultValue ? defaultValue.split('').slice(0, length) : Array.from({ length }, () => ''), @@ -40,7 +40,7 @@ export const useCodeControl = (formControl: FormControlState, options?: UseCodeI } }, [values.toString()]); - const otpInputProps = { length, values, setValues, errorText, ref: otpControlRef }; + const otpInputProps = { length, values, setValues, feedback, feedbackType, ref: otpControlRef }; return { otpInputProps, onCodeEntryFinished, reset: () => otpControlRef.current?.reset() }; }; @@ -55,7 +55,7 @@ export const CodeControl = React.forwardRef<{ reset: any }, CodeControlProps>((p const [disabled, setDisabled] = React.useState(false); const refs = React.useRef>([]); const firstClickRef = React.useRef(false); - const { values, setValues, isDisabled, errorText, isSuccessfullyFilled, isLoading, length } = props; + const { values, setValues, isDisabled, feedback, feedbackType, isSuccessfullyFilled, isLoading, length } = props; React.useImperativeHandle(ref, () => ({ reset: () => { @@ -70,10 +70,10 @@ export const CodeControl = React.forwardRef<{ reset: any }, CodeControlProps>((p }, []); React.useEffect(() => { - if (errorText) { + if (feedback) { setDisabled(true); } - }, [errorText]); + }, [feedback]); const handleMultipleCharValue = ({ eventValue, inputPosition }: { eventValue: string; inputPosition: number }) => { const eventValues = (eventValue || '').split(''); @@ -168,12 +168,12 @@ export const CodeControl = React.forwardRef<{ reset: any }, CodeControlProps>((p ((p autoComplete='one-time-code' aria-label={`${index === 0 ? 'Enter verification code. ' : ''} Digit ${index + 1}`} isDisabled={isDisabled || isLoading || disabled || isSuccessfullyFilled} - hasError={!!errorText} + hasError={feedbackType === 'error'} isSuccessfullyFilled={isSuccessfullyFilled} type='text' inputMode='numeric' @@ -209,7 +209,8 @@ export const CodeControl = React.forwardRef<{ reset: any }, CodeControlProps>((p )} , 'label' | 'placehol id: FieldId; isRequired?: boolean; isOptional?: boolean; - errorText?: string; onActionClicked?: React.MouseEventHandler; isDisabled?: boolean; label?: string | LocalizationKey; @@ -46,21 +46,19 @@ type FormControlProps = Omit, 'label' | 'placehol icon?: React.ComponentType; validatePassword?: boolean; setError: (error: string | ClerkAPIError | undefined) => void; - warningText: string | undefined; - setWarning: (error: string) => void; - setSuccessful: (message: string) => void; - successfulText: string; - hasLostFocus: boolean; + setWarning: (warning: string) => void; + setInfo: (info: string) => void; + setSuccess: (message: string) => void; + feedback: string; + feedbackType: FeedbackType; setHasPassedComplexity: (b: boolean) => void; hasPassedComplexity: boolean; - enableErrorAfterBlur?: boolean; - informationText?: string | LocalizationKey; + infoText?: string | LocalizationKey; radioOptions?: { value: string; label: string | LocalizationKey; description?: string | LocalizationKey; }[]; - isFocused: boolean; groupPreffix?: string; groupSuffix?: string; }; @@ -106,7 +104,7 @@ function useFormTextAnimation() { animation: `${enterAnimation ? animations.inAnimation : animations.outAnimation} ${ t.transitionDuration.$textField } ${t.transitionTiming.$common}`, - transition: `height ${t.transitionDuration.$slow} ${t.transitionTiming.$common}`, // This is expensive but required for a smooth layout shift + transition: `height ${t.transitionDuration.$slow} ${t.transitionTiming.$common}`, // This is expensive but required for a smooth layout shift }); }, [prefersReducedMotion], @@ -117,25 +115,18 @@ function useFormTextAnimation() { }; } -type CalculateConfigProps = { - recalculate?: LocalizationKey | string | undefined; -}; -type Px = number; -const useCalculateErrorTextHeight = (config: CalculateConfigProps = {}) => { - const [height, setHeight] = useState(24); +const useCalculateErrorTextHeight = ({ feedback }: { feedback: string }) => { + const [height, setHeight] = useState(0); const calculateHeight = useCallback( (element: HTMLElement | null) => { if (element) { - const computedStyles = getComputedStyle(element); - const marginTop = parseInt(computedStyles.marginTop.replace('px', '')); - - const newHeight = 1.1 * marginTop + element.scrollHeight; - setHeight(newHeight); + setHeight(element.scrollHeight); } }, - [config?.recalculate], + [feedback], ); + return { height, calculateHeight, @@ -144,25 +135,26 @@ const useCalculateErrorTextHeight = (config: CalculateConfigProps = {}) => { type FormFeedbackDescriptorsKeys = 'error' | 'warning' | 'info' | 'success'; +type Feedback = { feedback?: string; feedbackType?: FeedbackType; shouldEnter: boolean }; + type FormFeedbackProps = Partial['debounced'] & { id: FieldId }> & { elementDescriptors?: Partial>; }; -const delay = 350; - export const FormFeedback = (props: FormFeedbackProps) => { - const { id, elementDescriptors } = props; - const errorMessage = useFieldMessageVisibility(props.errorText, delay); - const successMessage = useFieldMessageVisibility(props.successfulText, delay); - const informationMessage = useFieldMessageVisibility(props.informationText, delay); - const warningMessage = useFieldMessageVisibility(props.warningText, delay); - - const messageToDisplay = informationMessage || successMessage || errorMessage || warningMessage; - const isSomeMessageVisible = !!messageToDisplay; - - const { calculateHeight, height } = useCalculateErrorTextHeight({ - recalculate: warningMessage || errorMessage || informationMessage, + const { id, elementDescriptors, feedback, feedbackType = 'info' } = props; + const [feedbacks, setFeedbacks] = useState<{ + a?: Feedback; + b?: Feedback; + }>({ a: { feedback, feedbackType, shouldEnter: true }, b: undefined }); + const { calculateHeight: calculateHeightA, height: heightA } = useCalculateErrorTextHeight({ + feedback: feedbacks.a?.feedback || '', }); + const { calculateHeight: calculateHeightB, height: heightB } = useCalculateErrorTextHeight({ + feedback: feedbacks.b?.feedback || '', + }); + const [heightMax, setHeightMax] = useState(Math.max(heightA, heightB)); + const { getFormTextAnimation } = useFormTextAnimation(); const defaultElementDescriptors = { error: descriptors.formFieldErrorText, @@ -171,6 +163,34 @@ export const FormFeedback = (props: FormFeedbackProps) => { success: descriptors.formFieldSuccessText, }; + useEffect(() => { + setFeedbacks(oldFeedbacks => { + if (oldFeedbacks.a?.shouldEnter) { + return { + a: { ...oldFeedbacks.a, shouldEnter: false }, + b: { + feedback, + feedbackType, + shouldEnter: true, + }, + }; + } else { + return { + a: { + feedback, + feedbackType, + shouldEnter: true, + }, + b: { ...oldFeedbacks.b, shouldEnter: false }, + }; + } + }); + }, [feedback, feedbackType]); + + useEffect(() => { + setHeightMax(h => Math.max(heightA, heightB, h)); + }, [heightA, heightB]); + const getElementProps = (type: FormFeedbackDescriptorsKeys) => { const descriptor = (elementDescriptors?.[type] || defaultElementDescriptors[type]) as ElementDescriptor | undefined; return { @@ -179,59 +199,51 @@ export const FormFeedback = (props: FormFeedbackProps) => { }; }; - if (!isSomeMessageVisible) { - return null; - } + const FormInfoComponent: Record< + FeedbackType, + typeof FormErrorText | typeof FormInfoText | typeof FormSuccessText | typeof FormWarningText + > = { + error: FormErrorText, + info: FormInfoText, + success: FormSuccessText, + warning: FormWarningText, + }; + + const InfoComponentA = FormInfoComponent[feedbacks.a?.feedbackType || 'info']; + const InfoComponentB = FormInfoComponent[feedbacks.b?.feedbackType || 'info']; return ( - {/*Display the directions after the success message is unmounted*/} - {!successMessage && !warningMessage && !errorMessage && informationMessage && ( - - )} - {/* Display the error message after the directions is unmounted*/} - {errorMessage && ( - + ({ + visibility: feedbacks.a?.shouldEnter ? 'visible' : 'hidden', + }), + getFormTextAnimation(!!feedbacks.a?.shouldEnter), + ]} + localizationKey={feedbacks.a?.feedback} /> - )} - - {/* Display the success message after the error message is unmounted*/} - {!errorMessage && successMessage && ( - ({ + visibility: feedbacks.b?.shouldEnter ? 'visible' : 'hidden', + }), + getFormTextAnimation(!!feedbacks.b?.shouldEnter), + ]} + localizationKey={feedbacks.b?.feedback} /> - )} - - {warningMessage && ( - - {warningMessage} - - )} + ); }; @@ -239,10 +251,12 @@ export const FormFeedback = (props: FormFeedbackProps) => { export const FormControl = forwardRef>((props, ref) => { const { t } = useLocalizations(); const card = useCardState(); - const { submittedWithEnter } = useFormState(); + const [isFocused, setIsFocused] = useState(false); const { + // eslint-disable-next-line @typescript-eslint/no-unused-vars + hasPassedComplexity, + infoText, id, - errorText, isRequired, isOptional, label, @@ -252,16 +266,12 @@ export const FormControl = forwardRef { + inputElementProps.onFocus?.(e); + setIsFocused(true); + }} + onBlur={e => { + inputElementProps.onBlur?.(e); + // set a timeout because new errors might appear + // and we don't want to spam layout shifts + setTimeout(() => { + setIsFocused(false); + }, 500); + }} ref={ref} placeholder={t(placeholder)} /> @@ -401,12 +411,13 @@ export const FormControl = forwardRef diff --git a/packages/clerk-js/src/ui/elements/PasswordInput.tsx b/packages/clerk-js/src/ui/elements/PasswordInput.tsx index 35d2cf7ed5a..f530368c932 100644 --- a/packages/clerk-js/src/ui/elements/PasswordInput.tsx +++ b/packages/clerk-js/src/ui/elements/PasswordInput.tsx @@ -1,4 +1,6 @@ import type { ChangeEvent } from 'react'; +import { useState } from 'react'; +import { useRef } from 'react'; import React, { forwardRef } from 'react'; import { useEnvironment } from '../contexts'; @@ -7,15 +9,20 @@ import { usePassword } from '../hooks/usePassword'; import { Eye, EyeSlash } from '../icons'; import { useFormControl } from '../primitives/hooks'; import type { PropsOfComponent } from '../styledSystem'; +import { mergeRefs } from '../utils'; import { IconButton } from './IconButton'; type PasswordInputProps = PropsOfComponent & { validatePassword?: boolean; }; +const DEBOUNCE_MS = 350; + export const PasswordInput = forwardRef((props, ref) => { const [hidden, setHidden] = React.useState(true); - const { id, onChange: onChangeProp, validatePassword = false, ...rest } = props; + const { id, onChange: onChangeProp, validatePassword: validatePasswordProp = false, ...rest } = props; + const inputRef = useRef(null); + const [timeoutState, setTimeoutState] = useState(null); const { userSettings: { passwordSettings }, @@ -24,19 +31,33 @@ export const PasswordInput = forwardRef((p const formControlProps = useFormControl(); const { t } = useLocalizations(); - const { setPassword } = usePassword( - { ...passwordSettings, validatePassword }, + const { validatePassword } = usePassword( + { ...passwordSettings, validatePassword: validatePasswordProp }, { onValidationSuccess: () => - formControlProps?.setSuccessful?.(t(localizationKeys('unstable__errors.zxcvbn.goodPassword'))), - onValidationFailed: errorMessage => formControlProps?.setError?.(errorMessage), - onValidationWarning: errorMessage => formControlProps?.setWarning?.(errorMessage), + formControlProps?.setSuccess?.(t(localizationKeys('unstable__errors.zxcvbn.goodPassword'))), + onValidationError: message => formControlProps?.setError?.(message), + onValidationWarning: message => formControlProps?.setWarning?.(message), + onValidationInfo: message => { + if (inputRef.current === document.activeElement) { + formControlProps?.setInfo?.(message); + } else { + formControlProps?.setError?.(message); + } + }, onValidationComplexity: hasPassed => formControlProps?.setHasPassedComplexity?.(hasPassed), }, ); const onChange = (e: ChangeEvent) => { - setPassword(e.target.value); + if (timeoutState) { + clearTimeout(timeoutState); + } + setTimeoutState( + setTimeout(() => { + validatePassword(e.target.value); + }, DEBOUNCE_MS), + ); return onChangeProp?.(e); }; @@ -50,7 +71,16 @@ export const PasswordInput = forwardRef((p { + rest.onBlur?.(e); + onChange(e); + }} + onFocus={e => { + rest.onFocus?.(e); + onChange(e); + }} + //@ts-expect-error + ref={mergeRefs(ref, inputRef)} type={hidden ? 'password' : 'text'} sx={theme => ({ paddingRight: theme.space.$10 })} /> diff --git a/packages/clerk-js/src/ui/hooks/usePassword.ts b/packages/clerk-js/src/ui/hooks/usePassword.ts index c3741e1ee9f..4c8126d1d79 100644 --- a/packages/clerk-js/src/ui/hooks/usePassword.ts +++ b/packages/clerk-js/src/ui/hooks/usePassword.ts @@ -11,9 +11,10 @@ import { generateErrorTextUtil } from './usePasswordComplexity'; export const usePassword = (config: UsePasswordConfig, callbacks?: UsePasswordCbs) => { const { t, locale } = useLocalizations(); const { - onValidationFailed = noop, + onValidationError = noop, onValidationSuccess = noop, onValidationWarning = noop, + onValidationInfo = noop, onValidationComplexity, } = callbacks || {}; @@ -23,14 +24,18 @@ export const usePassword = (config: UsePasswordConfig, callbacks?: UsePasswordCb * Failed complexity rules always have priority */ if (Object.values(res?.complexity || {}).length > 0) { - return onValidationFailed( - generateErrorTextUtil({ - config, - t, - failedValidations: res.complexity, - locale, - }), - ); + const message = generateErrorTextUtil({ + config, + t, + failedValidations: res.complexity, + locale, + }); + + if (res.complexity?.min_length) { + return onValidationInfo(message); + } + + return onValidationError(message); } /** @@ -38,7 +43,7 @@ export const usePassword = (config: UsePasswordConfig, callbacks?: UsePasswordCb */ if (res?.strength?.state === 'fail') { const error = res.strength.keys.map(localizationKey => t(localizationKeys(localizationKey as any))).join(' '); - return onValidationFailed(error); + return onValidationError(error); } /** @@ -57,7 +62,7 @@ export const usePassword = (config: UsePasswordConfig, callbacks?: UsePasswordCb [callbacks, t, locale], ); - const setPassword = useMemo(() => { + const validatePassword = useMemo(() => { return createValidatePassword(config, { onValidation: onValidate, onValidationComplexity, @@ -65,7 +70,7 @@ export const usePassword = (config: UsePasswordConfig, callbacks?: UsePasswordCb }, [onValidate]); return { - setPassword, + validatePassword, }; }; @@ -90,12 +95,12 @@ export const useConfirmPassword = ({ const displayConfirmPasswordFeedback = useCallback( (password: string) => { if (checkPasswordMatch(password)) { - confirmPasswordField.setSuccessful(t(localizationKeys('formFieldError__matchingPasswords'))); + confirmPasswordField.setSuccess(t(localizationKeys('formFieldError__matchingPasswords'))); } else { confirmPasswordField.setError(t(localizationKeys('formFieldError__notMatchingPasswords'))); } }, - [confirmPasswordField.setError, confirmPasswordField.setSuccessful, t, checkPasswordMatch], + [confirmPasswordField.setError, confirmPasswordField.setSuccess, t, checkPasswordMatch], ); return { diff --git a/packages/clerk-js/src/ui/hooks/useSetTimeout.ts b/packages/clerk-js/src/ui/hooks/useSetTimeout.ts index 9e9973ae7d1..dc34549a122 100644 --- a/packages/clerk-js/src/ui/hooks/useSetTimeout.ts +++ b/packages/clerk-js/src/ui/hooks/useSetTimeout.ts @@ -9,7 +9,7 @@ export function useSetTimeout(value: T, delayInMs?: number): T { return () => { clearTimeout(timer); }; - }, [value, delayInMs]); + }, [JSON.stringify(value), delayInMs]); return timer; } diff --git a/packages/clerk-js/src/ui/primitives/FormControl.tsx b/packages/clerk-js/src/ui/primitives/FormControl.tsx index b3a38155625..44b15ac6466 100644 --- a/packages/clerk-js/src/ui/primitives/FormControl.tsx +++ b/packages/clerk-js/src/ui/primitives/FormControl.tsx @@ -5,7 +5,8 @@ import type { FormControlProps } from './hooks'; import { FormControlContextProvider } from './hooks'; export const FormControl = (props: React.PropsWithChildren) => { - const { hasError, id, isRequired, setError, setSuccessful, setWarning, setHasPassedComplexity, ...rest } = props; + const { hasError, id, isRequired, setError, setInfo, setSuccess, setWarning, setHasPassedComplexity, ...rest } = + props; return ( ) => css={{ position: 'relative', flex: '1 1 auto' }} > {props.children} diff --git a/packages/clerk-js/src/ui/primitives/hooks/useFormControl.tsx b/packages/clerk-js/src/ui/primitives/hooks/useFormControl.tsx index 0fb370722f5..6ec92a08599 100644 --- a/packages/clerk-js/src/ui/primitives/hooks/useFormControl.tsx +++ b/packages/clerk-js/src/ui/primitives/hooks/useFormControl.tsx @@ -12,8 +12,9 @@ export type FormControlProps = { hasError?: boolean; isDisabled?: boolean; setError: (error: string | ClerkAPIError | undefined) => void; - setSuccessful: (message: string) => void; - setWarning: (message: string) => void; + setSuccess: (message: string) => void; + setWarning: (warning: string) => void; + setInfo: (info: string) => void; setHasPassedComplexity: (b: boolean) => void; }; @@ -29,8 +30,9 @@ export const FormControlContextProvider = (props: React.PropsWithChildren{props.children}; }; diff --git a/packages/clerk-js/src/ui/styledSystem/animations.ts b/packages/clerk-js/src/ui/styledSystem/animations.ts index 4dcf904cda5..e2c643bbcc9 100644 --- a/packages/clerk-js/src/ui/styledSystem/animations.ts +++ b/packages/clerk-js/src/ui/styledSystem/animations.ts @@ -65,20 +65,17 @@ const notificationAnimation = keyframes` `; const outAnimation = keyframes` - 20% { - opacity: 1; - transform: translateY(0px); + 0% { + opacity:1; + translateY(0px); max-height: 6rem; - } - 80% { - opacity: 0; - transform: translateY(5px); - max-height: 0; - } + visibility: visible; + } 100% { opacity: 0; transform: translateY(5px); max-height: 0; + visibility: visible; } `; diff --git a/packages/clerk-js/src/ui/utils/useFormControl.ts b/packages/clerk-js/src/ui/utils/useFormControl.ts index dac0e1c08b1..3a9c061b581 100644 --- a/packages/clerk-js/src/ui/utils/useFormControl.ts +++ b/packages/clerk-js/src/ui/utils/useFormControl.ts @@ -1,6 +1,6 @@ import type { ClerkAPIError } from '@clerk/types'; import type { HTMLInputTypeAttribute } from 'react'; -import React, { useMemo } from 'react'; +import { useState } from 'react'; import { useSetTimeout } from '../hooks'; import type { LocalizationKey } from '../localization'; @@ -13,8 +13,7 @@ type Options = { placeholder?: string | LocalizationKey; options?: SelectOption[]; defaultChecked?: boolean; - enableErrorAfterBlur?: boolean; - informationText?: string | LocalizationKey; + infoText?: LocalizationKey | string; } & ( | { label: string | LocalizationKey; @@ -49,28 +48,27 @@ type FieldStateProps = { value: string; checked?: boolean; onChange: React.ChangeEventHandler; - onBlur: React.FocusEventHandler; - onFocus: React.FocusEventHandler; - hasLostFocus: boolean; - errorText: string | undefined; - warningText: string; + feedback: string; + feedbackType: FeedbackType; setError: (error: string | ClerkAPIError | undefined) => void; - setWarning: (message: string) => void; - setSuccessful: (message: string) => void; + setWarning: (warning: string) => void; + setSuccess: (message: string) => void; + setInfo: (info: string) => void; setHasPassedComplexity: (b: boolean) => void; hasPassedComplexity: boolean; - successfulText: string; - isFocused: boolean; } & Omit; export type FormControlState = FieldStateProps & { setError: (error: string | ClerkAPIError | undefined) => void; - setSuccessful: (message: string) => void; + setSuccess: (message: string) => void; + setInfo: (info: string) => void; setValue: (val: string | undefined) => void; setChecked: (isChecked: boolean) => void; props: FieldStateProps; }; +export type FeedbackType = 'success' | 'error' | 'warning' | 'info'; + export const useFormControl = ( id: Id, initialState: string, @@ -82,20 +80,17 @@ export const useFormControl = ( isRequired: false, placeholder: '', options: [], - enableErrorAfterBlur: false, - informationText: '', defaultChecked: false, }; - const { translateError } = useLocalizations(); - const [value, setValueInternal] = React.useState(initialState); - const [checked, setCheckedInternal] = React.useState(opts?.defaultChecked || false); - const [errorText, setErrorText] = React.useState(undefined); - const [warningText, setWarningText] = React.useState(''); - const [successfulText, setSuccessfulText] = React.useState(''); - const [hasLostFocus, setHasLostFocus] = React.useState(false); - const [isFocused, setFocused] = React.useState(false); - const [hasPassedComplexity, setHasPassedComplexity] = React.useState(false); + const { translateError, t } = useLocalizations(); + const [value, setValueInternal] = useState(initialState); + const [checked, setCheckedInternal] = useState(opts?.defaultChecked || false); + const [hasPassedComplexity, setHasPassedComplexity] = useState(false); + const [feedback, setFeedback] = useState<{ message: string; type: FeedbackType }>({ + message: '', + type: 'info', + }); const onChange: FormControlState['onChange'] = event => { if (opts?.type === 'checkbox') { @@ -104,35 +99,28 @@ export const useFormControl = ( return setValueInternal(event.target.value || ''); }; - const onFocus: FormControlState['onFocus'] = () => { - setFocused(true); - }; - - const onBlur: FormControlState['onBlur'] = () => { - setFocused(false); - setHasLostFocus(true); - }; - const setValue: FormControlState['setValue'] = val => setValueInternal(val || ''); const setChecked: FormControlState['setChecked'] = checked => setCheckedInternal(checked); const setError: FormControlState['setError'] = error => { - setErrorText(translateError(error || undefined)); - if (typeof error !== 'undefined') { - setSuccessfulText(''); - setWarningText(''); + if (error) { + setFeedback({ message: translateError(error), type: 'error' }); } }; - const setSuccessful: FormControlState['setSuccessful'] = isSuccess => { - setErrorText(''); - setWarningText(''); - setSuccessfulText(isSuccess); + const setSuccess: FormControlState['setSuccess'] = message => { + if (message) { + setFeedback({ message, type: 'success' }); + } }; const setWarning: FormControlState['setWarning'] = warning => { - setWarningText(warning); if (warning) { - setSuccessfulText(''); - setErrorText(''); + setFeedback({ message: translateError(warning), type: 'warning' }); + } + }; + + const setInfo: FormControlState['setInfo'] = info => { + if (info) { + setFeedback({ message: translateError(info), type: 'info' }); } }; @@ -144,18 +132,13 @@ export const useFormControl = ( name: id, value, checked, - errorText, - successfulText, - hasLostFocus, - setSuccessful, + setSuccess, setError, onChange, - onBlur, - onFocus, - isFocused, - enableErrorAfterBlur: restOpts.enableErrorAfterBlur || false, setWarning, - warningText, + feedback: feedback.message || t(opts.infoText), + feedbackType: feedback.type, + setInfo, hasPassedComplexity, setHasPassedComplexity, validatePassword: opts.type === 'password' ? opts.validatePassword : undefined, @@ -177,82 +160,26 @@ export const buildRequest = (fieldStates: Array): Record { - const { - hasLostFocus = false, - errorText = '', - warningText = '', - enableErrorAfterBlur = false, - successfulText = '', - isFocused = false, - informationText = '', - skipBlur = false, - delayInMs = 100, - } = opts; - - const canDisplayFeedback = useMemo(() => { - if (enableErrorAfterBlur) { - if (skipBlur) { - return true; - } - return hasLostFocus; - } - return true; - }, [enableErrorAfterBlur, hasLostFocus, skipBlur]); - - const feedbackMemo = useMemo(() => { - const shouldDisplayErrorAsWarning = errorText && !skipBlur; - const _errorText = !shouldDisplayErrorAsWarning && canDisplayFeedback ? errorText : ''; - const _warningText = shouldDisplayErrorAsWarning ? errorText : warningText; - const _successfulText = successfulText; +export const useFormControlFeedback = (opts?: DebouncingOption): DebouncedFeedback => { + const { feedback = '', delayInMs = 100, feedbackType = 'info', isFocused = false } = opts || {}; - /* - * On keyboard navigation avoid displaying the information text when an error is present. - * This is necessary in order to ensure that users will still be able to see the error message - * even if they have pressed Enter (to submit form) and field still has focus. - */ - const shouldShowInformationText = skipBlur - ? isFocused && !_successfulText && !_errorText - : isFocused && !_successfulText; - return { - errorText: _errorText, - successfulText: _successfulText, - warningText: _warningText, - isFocused, - informationText: shouldShowInformationText ? informationText : '', - }; - }, [ - informationText, - enableErrorAfterBlur, - isFocused, - successfulText, - hasLostFocus, - errorText, - canDisplayFeedback, - skipBlur, - ]); + const shouldHide = isFocused ? false : ['info', 'warning'].includes(feedbackType); - const debouncedState = useSetTimeout(feedbackMemo, delayInMs); + const debouncedState = useSetTimeout( + { feedback: shouldHide ? '' : feedback, feedbackType: shouldHide ? 'info' : feedbackType }, + delayInMs, + ); return { debounced: debouncedState, diff --git a/packages/clerk-js/src/utils/passwords/password.ts b/packages/clerk-js/src/utils/passwords/password.ts index 5f145077253..9abc9e17697 100644 --- a/packages/clerk-js/src/utils/passwords/password.ts +++ b/packages/clerk-js/src/utils/passwords/password.ts @@ -11,9 +11,10 @@ export type UsePasswordConfig = PasswordSettingsData & { }; export type UsePasswordCbs = { - onValidationFailed?: (errorMessage: string | undefined) => void; + onValidationError?: (error: string | undefined) => void; onValidationSuccess?: () => void; - onValidationWarning?: (warningMessage: string) => void; + onValidationWarning?: (warning: string) => void; + onValidationInfo?: (info: string) => void; onValidationComplexity?: (b: boolean) => void; }; From 8173f980df9baa9a8fd1633104b640521240ebce Mon Sep 17 00:00:00 2001 From: George Desipris Date: Thu, 28 Sep 2023 12:57:53 +0300 Subject: [PATCH 2/3] fix(clerk-js): Form error animation polishing test(clerk-js): Fix tests for password validation chore(clerk-js): Changeset chore(clerk-js): Improve changeset fix(clerk-js): Address PR comments refactor(clerk-js): Avoid useEffect usage for feedbacks and maxHeight fix(clerk-js): Add clearFeedback to types fix(clerk-js): Delay feedback after focus/blur on input elements --- .changeset/slimy-singers-glow.md | 5 + packages/clerk-js/src/core/constants.ts | 2 + .../ui/components/SignIn/ResetPassword.tsx | 8 +- .../SignIn/__tests__/ResetPassword.test.tsx | 5 +- .../components/UserProfile/PasswordPage.tsx | 8 +- .../__tests__/PasswordPage.test.tsx | 9 +- .../clerk-js/src/ui/elements/FormControl.tsx | 147 ++++++++++-------- .../src/ui/elements/PasswordInput.tsx | 3 +- packages/clerk-js/src/ui/hooks/index.ts | 2 +- packages/clerk-js/src/ui/hooks/useDebounce.ts | 32 ++++ .../src/ui/hooks/useDelayedVisibility.ts | 3 +- packages/clerk-js/src/ui/hooks/usePassword.ts | 4 +- .../clerk-js/src/ui/hooks/useSetTimeout.ts | 15 -- .../src/ui/primitives/FormControl.tsx | 26 +++- .../ui/primitives/hooks/useFormControl.tsx | 4 + .../src/ui/styledSystem/animations.ts | 19 +++ .../clerk-js/src/ui/utils/test/fixtures.ts | 15 ++ .../clerk-js/src/ui/utils/useFormControl.ts | 18 ++- 18 files changed, 217 insertions(+), 108 deletions(-) create mode 100644 .changeset/slimy-singers-glow.md create mode 100644 packages/clerk-js/src/ui/hooks/useDebounce.ts delete mode 100644 packages/clerk-js/src/ui/hooks/useSetTimeout.ts diff --git a/.changeset/slimy-singers-glow.md b/.changeset/slimy-singers-glow.md new file mode 100644 index 00000000000..f116930380c --- /dev/null +++ b/.changeset/slimy-singers-glow.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': minor +--- + +Greatly improve the UX when users are creating their passwords. The hints below the input fields now have smoother animations and show more types of feedback based on different conditions. Additionally, the password validation is now debounced. diff --git a/packages/clerk-js/src/core/constants.ts b/packages/clerk-js/src/core/constants.ts index ffa65e9171c..66416ee8f50 100644 --- a/packages/clerk-js/src/core/constants.ts +++ b/packages/clerk-js/src/core/constants.ts @@ -20,3 +20,5 @@ export const ERROR_CODES = { export const SIGN_IN_INITIAL_VALUE_KEYS = ['email_address', 'phone_number', 'username']; export const SIGN_UP_INITIAL_VALUE_KEYS = ['email_address', 'phone_number', 'username', 'first_name', 'last_name']; + +export const DEBOUNCE_MS = 350; diff --git a/packages/clerk-js/src/ui/components/SignIn/ResetPassword.tsx b/packages/clerk-js/src/ui/components/SignIn/ResetPassword.tsx index a9f413d4bd9..27b80d34cb3 100644 --- a/packages/clerk-js/src/ui/components/SignIn/ResetPassword.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/ResetPassword.tsx @@ -52,7 +52,7 @@ export const _ResetPassword = () => { defaultChecked: true, }); - const { displayConfirmPasswordFeedback, isPasswordMatch } = useConfirmPassword({ + const { setConfirmPasswordFeedback, isPasswordMatch } = useConfirmPassword({ passwordField, confirmPasswordField: confirmField, }); @@ -61,7 +61,7 @@ export const _ResetPassword = () => { const validateForm = () => { if (passwordField.value) { - displayConfirmPasswordFeedback(confirmField.value); + setConfirmPasswordFeedback(confirmField.value); } }; @@ -127,7 +127,9 @@ export const _ResetPassword = () => { { - displayConfirmPasswordFeedback(e.target.value); + if (e.target.value) { + setConfirmPasswordFeedback(e.target.value); + } return confirmField.props.onChange(e); }} /> diff --git a/packages/clerk-js/src/ui/components/SignIn/__tests__/ResetPassword.test.tsx b/packages/clerk-js/src/ui/components/SignIn/__tests__/ResetPassword.test.tsx index b2f9e58c0fa..e2df2fc4637 100644 --- a/packages/clerk-js/src/ui/components/SignIn/__tests__/ResetPassword.test.tsx +++ b/packages/clerk-js/src/ui/components/SignIn/__tests__/ResetPassword.test.tsx @@ -118,14 +118,13 @@ describe('ResetPassword', () => { await userEvent.type(screen.getByLabelText(/new password/i), 'testewrewr'); const confirmField = screen.getByLabelText(/confirm password/i); await userEvent.type(confirmField, 'testrwerrwqrwe'); - fireEvent.blur(confirmField); await waitFor(() => { - screen.getByText(`Passwords don't match.`); + expect(screen.getByText(`Passwords don't match.`)).toBeInTheDocument(); }); await userEvent.clear(confirmField); await waitFor(() => { - screen.getByText(`Passwords don't match.`); + expect(screen.getByText(`Passwords don't match.`)).toBeInTheDocument(); }); }); }, 10000); diff --git a/packages/clerk-js/src/ui/components/UserProfile/PasswordPage.tsx b/packages/clerk-js/src/ui/components/UserProfile/PasswordPage.tsx index c604d8564ed..394c78c5f30 100644 --- a/packages/clerk-js/src/ui/components/UserProfile/PasswordPage.tsx +++ b/packages/clerk-js/src/ui/components/UserProfile/PasswordPage.tsx @@ -80,7 +80,7 @@ export const PasswordPage = withCardStateProvider(() => { defaultChecked: true, }); - const { displayConfirmPasswordFeedback, isPasswordMatch } = useConfirmPassword({ + const { setConfirmPasswordFeedback, isPasswordMatch } = useConfirmPassword({ passwordField, confirmPasswordField: confirmField, }); @@ -94,7 +94,7 @@ export const PasswordPage = withCardStateProvider(() => { const validateForm = () => { if (passwordField.value) { - displayConfirmPasswordFeedback(confirmField.value); + setConfirmPasswordFeedback(confirmField.value); } }; @@ -165,7 +165,9 @@ export const PasswordPage = withCardStateProvider(() => { { - displayConfirmPasswordFeedback(e.target.value); + if (e.target.value) { + setConfirmPasswordFeedback(e.target.value); + } return confirmField.props.onChange(e); }} isDisabled={passwordEditDisabled} diff --git a/packages/clerk-js/src/ui/components/UserProfile/__tests__/PasswordPage.test.tsx b/packages/clerk-js/src/ui/components/UserProfile/__tests__/PasswordPage.test.tsx index 0f11439f825..40a6bc55d12 100644 --- a/packages/clerk-js/src/ui/components/UserProfile/__tests__/PasswordPage.test.tsx +++ b/packages/clerk-js/src/ui/components/UserProfile/__tests__/PasswordPage.test.tsx @@ -269,22 +269,25 @@ describe('PasswordPage', () => { it(`Displays "Password match" when password match and removes it if they stop`, async () => { const { wrapper } = await createFixtures(initConfig); + await runFakeTimers(async () => { const { userEvent } = render(, { wrapper }); const passwordField = screen.getByLabelText(/new password/i); await userEvent.type(passwordField, 'testewrewr'); const confirmField = screen.getByLabelText(/confirm password/i); - expect(screen.queryByText(`Passwords match.`)).not.toBeInTheDocument(); + await waitFor(() => { + expect(screen.queryByText(`Passwords match.`)).not.toBeInTheDocument(); + }); await userEvent.type(confirmField, 'testewrewr'); await waitFor(() => { - screen.getByText(`Passwords match.`); + expect(screen.getByText(`Passwords match.`)).toBeInTheDocument(); }); await userEvent.type(confirmField, 'testrwerrwqrwe'); await waitFor(() => { - expect(screen.queryByText(`Passwords match.`)).not.toBeInTheDocument(); + expect(screen.queryByText(`Passwords match.`)).not.toBeVisible(); }); await userEvent.type(passwordField, 'testrwerrwqrwe'); diff --git a/packages/clerk-js/src/ui/elements/FormControl.tsx b/packages/clerk-js/src/ui/elements/FormControl.tsx index 9763bfdd0e8..46b47435eeb 100644 --- a/packages/clerk-js/src/ui/elements/FormControl.tsx +++ b/packages/clerk-js/src/ui/elements/FormControl.tsx @@ -1,7 +1,7 @@ import type { FieldId } from '@clerk/types'; import type { ClerkAPIError } from '@clerk/types'; import type { PropsWithChildren } from 'react'; -import { useEffect } from 'react'; +import { useRef } from 'react'; import React, { forwardRef, useCallback, useMemo, useState } from 'react'; import type { LocalizationKey } from '../customizables'; @@ -52,6 +52,7 @@ type FormControlProps = Omit, 'label' | 'placehol feedback: string; feedbackType: FeedbackType; setHasPassedComplexity: (b: boolean) => void; + clearFeedback: () => void; hasPassedComplexity: boolean; infoText?: string | LocalizationKey; radioOptions?: { @@ -94,16 +95,20 @@ function useFormTextAnimation() { const prefersReducedMotion = usePrefersReducedMotion(); const getFormTextAnimation = useCallback( - (enterAnimation: boolean): ThemableCssProp => { + (enterAnimation: boolean, options?: { inDelay?: boolean }): ThemableCssProp => { if (prefersReducedMotion) { return { animation: 'none', }; } return t => ({ - animation: `${enterAnimation ? animations.inAnimation : animations.outAnimation} ${ - t.transitionDuration.$textField - } ${t.transitionTiming.$common}`, + animation: `${ + enterAnimation + ? options?.inDelay + ? animations.inDelayAnimation + : animations.inAnimation + : animations.outAnimation + } ${t.transitionDuration.$textField} ${t.transitionTiming.$common}`, transition: `height ${t.transitionDuration.$slow} ${t.transitionTiming.$common}`, // This is expensive but required for a smooth layout shift }); }, @@ -121,7 +126,8 @@ const useCalculateErrorTextHeight = ({ feedback }: { feedback: string }) => { const calculateHeight = useCallback( (element: HTMLElement | null) => { if (element) { - setHeight(element.scrollHeight); + const computedStyles = getComputedStyle(element); + setHeight(element.scrollHeight + parseInt(computedStyles.marginTop.replace('px', ''))); } }, [feedback], @@ -143,17 +149,10 @@ type FormFeedbackProps = Partial['debo export const FormFeedback = (props: FormFeedbackProps) => { const { id, elementDescriptors, feedback, feedbackType = 'info' } = props; - const [feedbacks, setFeedbacks] = useState<{ + const feedbacksRef = useRef<{ a?: Feedback; b?: Feedback; }>({ a: { feedback, feedbackType, shouldEnter: true }, b: undefined }); - const { calculateHeight: calculateHeightA, height: heightA } = useCalculateErrorTextHeight({ - feedback: feedbacks.a?.feedback || '', - }); - const { calculateHeight: calculateHeightB, height: heightB } = useCalculateErrorTextHeight({ - feedback: feedbacks.b?.feedback || '', - }); - const [heightMax, setHeightMax] = useState(Math.max(heightA, heightB)); const { getFormTextAnimation } = useFormTextAnimation(); const defaultElementDescriptors = { @@ -163,32 +162,47 @@ export const FormFeedback = (props: FormFeedbackProps) => { success: descriptors.formFieldSuccessText, }; - useEffect(() => { - setFeedbacks(oldFeedbacks => { - if (oldFeedbacks.a?.shouldEnter) { - return { - a: { ...oldFeedbacks.a, shouldEnter: false }, - b: { - feedback, - feedbackType, - shouldEnter: true, - }, - }; - } else { - return { - a: { - feedback, - feedbackType, - shouldEnter: true, - }, - b: { ...oldFeedbacks.b, shouldEnter: false }, - }; - } - }); + const feedbacks = useMemo(() => { + const oldFeedbacks = feedbacksRef.current; + let result: { + a?: Feedback; + b?: Feedback; + }; + if (oldFeedbacks.a?.shouldEnter) { + result = { + a: { ...oldFeedbacks.a, shouldEnter: false }, + b: { + feedback, + feedbackType, + shouldEnter: true, + }, + }; + } else { + result = { + a: { + feedback, + feedbackType, + shouldEnter: true, + }, + b: { ...oldFeedbacks.b, shouldEnter: false }, + }; + } + feedbacksRef.current = result; + return result; }, [feedback, feedbackType]); - useEffect(() => { - setHeightMax(h => Math.max(heightA, heightB, h)); + const { calculateHeight: calculateHeightA, height: heightA } = useCalculateErrorTextHeight({ + feedback: feedbacks.a?.feedback || '', + }); + const { calculateHeight: calculateHeightB, height: heightB } = useCalculateErrorTextHeight({ + feedback: feedbacks.b?.feedback || '', + }); + const maxHeightRef = useRef(Math.max(heightA, heightB)); + + const maxHeight = useMemo(() => { + const max = Math.max(heightA, heightB, maxHeightRef.current); + maxHeightRef.current = max; + return max; }, [heightA, heightB]); const getElementProps = (type: FormFeedbackDescriptorsKeys) => { @@ -215,35 +229,33 @@ export const FormFeedback = (props: FormFeedbackProps) => { return ( - - ({ - visibility: feedbacks.a?.shouldEnter ? 'visible' : 'hidden', - }), - getFormTextAnimation(!!feedbacks.a?.shouldEnter), - ]} - localizationKey={feedbacks.a?.feedback} - /> - ({ - visibility: feedbacks.b?.shouldEnter ? 'visible' : 'hidden', - }), - getFormTextAnimation(!!feedbacks.b?.shouldEnter), - ]} - localizationKey={feedbacks.b?.feedback} - /> - + ({ + visibility: feedbacks.a?.shouldEnter ? 'visible' : 'hidden', + }), + getFormTextAnimation(!!feedbacks.a?.shouldEnter, { inDelay: true }), + ]} + localizationKey={feedbacks.a?.feedback} + /> + ({ + visibility: feedbacks.b?.shouldEnter ? 'visible' : 'hidden', + }), + getFormTextAnimation(!!feedbacks.b?.shouldEnter, { inDelay: true }), + ]} + localizationKey={feedbacks.b?.feedback} + /> ); }; @@ -255,6 +267,7 @@ export const FormControl = forwardRef { inputElementProps.onBlur?.(e); - // set a timeout because new errors might appear - // and we don't want to spam layout shifts - setTimeout(() => { - setIsFocused(false); - }, 500); + setIsFocused(false); }} ref={ref} placeholder={t(placeholder)} @@ -419,6 +429,7 @@ export const FormControl = forwardRef {isCheckbox ? ( diff --git a/packages/clerk-js/src/ui/elements/PasswordInput.tsx b/packages/clerk-js/src/ui/elements/PasswordInput.tsx index f530368c932..8f3341f5050 100644 --- a/packages/clerk-js/src/ui/elements/PasswordInput.tsx +++ b/packages/clerk-js/src/ui/elements/PasswordInput.tsx @@ -3,6 +3,7 @@ import { useState } from 'react'; import { useRef } from 'react'; import React, { forwardRef } from 'react'; +import { DEBOUNCE_MS } from '../../core/constants'; import { useEnvironment } from '../contexts'; import { descriptors, Flex, Input, localizationKeys, useLocalizations } from '../customizables'; import { usePassword } from '../hooks/usePassword'; @@ -16,8 +17,6 @@ type PasswordInputProps = PropsOfComponent & { validatePassword?: boolean; }; -const DEBOUNCE_MS = 350; - export const PasswordInput = forwardRef((props, ref) => { const [hidden, setHidden] = React.useState(true); const { id, onChange: onChangeProp, validatePassword: validatePasswordProp = false, ...rest } = props; diff --git a/packages/clerk-js/src/ui/hooks/index.ts b/packages/clerk-js/src/ui/hooks/index.ts index 26e551acccb..d0527e8808e 100644 --- a/packages/clerk-js/src/ui/hooks/index.ts +++ b/packages/clerk-js/src/ui/hooks/index.ts @@ -14,7 +14,7 @@ export * from './usePrefersReducedMotion'; export * from './useLocalStorage'; export * from './useSafeState'; export * from './useSearchInput'; -export * from './useSetTimeout'; +export * from './useDebounce'; export * from './useScrollLock'; export * from './useDeepEqualMemo'; export * from './useClerkModalStateParams'; diff --git a/packages/clerk-js/src/ui/hooks/useDebounce.ts b/packages/clerk-js/src/ui/hooks/useDebounce.ts new file mode 100644 index 00000000000..aea7086d789 --- /dev/null +++ b/packages/clerk-js/src/ui/hooks/useDebounce.ts @@ -0,0 +1,32 @@ +import { useEffect, useState } from 'react'; + +export function useDebounce(value: T, delayInMs?: number): T { + const [debouncedValue, setDebouncedValue] = useState(value); + const [timeoutState, setTimeoutState] = useState(undefined); + + useEffect(() => { + const handleDebounce = () => { + if (timeoutState) { + clearTimeout(timeoutState); + setTimeoutState(undefined); + } + + setTimeoutState( + setTimeout(() => { + setDebouncedValue(value); + setTimeoutState(undefined); + }, delayInMs || 500), + ); + }; + + handleDebounce(); + return () => { + if (timeoutState) { + clearTimeout(timeoutState); + setTimeoutState(undefined); + } + }; + }, [JSON.stringify(value), delayInMs]); + + return debouncedValue; +} diff --git a/packages/clerk-js/src/ui/hooks/useDelayedVisibility.ts b/packages/clerk-js/src/ui/hooks/useDelayedVisibility.ts index 49fd7373f5f..62713780bd3 100644 --- a/packages/clerk-js/src/ui/hooks/useDelayedVisibility.ts +++ b/packages/clerk-js/src/ui/hooks/useDelayedVisibility.ts @@ -6,7 +6,7 @@ import { useEffect, useState } from 'react'; * Immediate change for in-between changes */ export function useDelayedVisibility(valueToDelay: T, delayInMs: number) { - const [isVisible, setVisible] = useState(); + const [isVisible, setVisible] = useState(undefined); useEffect(() => { let timeoutId: ReturnType; @@ -23,6 +23,7 @@ export function useDelayedVisibility(valueToDelay: T, delayInMs: number) { } return () => clearTimeout(timeoutId); }, [valueToDelay, delayInMs, isVisible]); + return isVisible; } diff --git a/packages/clerk-js/src/ui/hooks/usePassword.ts b/packages/clerk-js/src/ui/hooks/usePassword.ts index 4c8126d1d79..1fcce876077 100644 --- a/packages/clerk-js/src/ui/hooks/usePassword.ts +++ b/packages/clerk-js/src/ui/hooks/usePassword.ts @@ -92,7 +92,7 @@ export const useConfirmPassword = ({ [checkPasswordMatch, confirmPasswordField.value], ); - const displayConfirmPasswordFeedback = useCallback( + const setConfirmPasswordFeedback = useCallback( (password: string) => { if (checkPasswordMatch(password)) { confirmPasswordField.setSuccess(t(localizationKeys('formFieldError__matchingPasswords'))); @@ -104,7 +104,7 @@ export const useConfirmPassword = ({ ); return { - displayConfirmPasswordFeedback, + setConfirmPasswordFeedback, isPasswordMatch, }; }; diff --git a/packages/clerk-js/src/ui/hooks/useSetTimeout.ts b/packages/clerk-js/src/ui/hooks/useSetTimeout.ts deleted file mode 100644 index dc34549a122..00000000000 --- a/packages/clerk-js/src/ui/hooks/useSetTimeout.ts +++ /dev/null @@ -1,15 +0,0 @@ -import { useEffect, useState } from 'react'; - -export function useSetTimeout(value: T, delayInMs?: number): T { - const [timer, setTimer] = useState(value); - - useEffect(() => { - const timer = setTimeout(() => setTimer(value), delayInMs || 500); - - return () => { - clearTimeout(timer); - }; - }, [JSON.stringify(value), delayInMs]); - - return timer; -} diff --git a/packages/clerk-js/src/ui/primitives/FormControl.tsx b/packages/clerk-js/src/ui/primitives/FormControl.tsx index 44b15ac6466..c62d70a56f1 100644 --- a/packages/clerk-js/src/ui/primitives/FormControl.tsx +++ b/packages/clerk-js/src/ui/primitives/FormControl.tsx @@ -5,8 +5,18 @@ import type { FormControlProps } from './hooks'; import { FormControlContextProvider } from './hooks'; export const FormControl = (props: React.PropsWithChildren) => { - const { hasError, id, isRequired, setError, setInfo, setSuccess, setWarning, setHasPassedComplexity, ...rest } = - props; + const { + hasError, + id, + isRequired, + setError, + setInfo, + clearFeedback, + setSuccess, + setWarning, + setHasPassedComplexity, + ...rest + } = props; return ( ) => css={{ position: 'relative', flex: '1 1 auto' }} > {props.children} diff --git a/packages/clerk-js/src/ui/primitives/hooks/useFormControl.tsx b/packages/clerk-js/src/ui/primitives/hooks/useFormControl.tsx index 6ec92a08599..ae03afec60c 100644 --- a/packages/clerk-js/src/ui/primitives/hooks/useFormControl.tsx +++ b/packages/clerk-js/src/ui/primitives/hooks/useFormControl.tsx @@ -16,6 +16,7 @@ export type FormControlProps = { setWarning: (warning: string) => void; setInfo: (info: string) => void; setHasPassedComplexity: (b: boolean) => void; + clearFeedback: () => void; }; type FormControlContextValue = Required & { errorMessageId: string }; @@ -34,6 +35,7 @@ export const FormControlContextProvider = (props: React.PropsWithChildren{props.children}; diff --git a/packages/clerk-js/src/ui/styledSystem/animations.ts b/packages/clerk-js/src/ui/styledSystem/animations.ts index e2c643bbcc9..5a20654fd4c 100644 --- a/packages/clerk-js/src/ui/styledSystem/animations.ts +++ b/packages/clerk-js/src/ui/styledSystem/animations.ts @@ -47,6 +47,24 @@ const inAnimation = keyframes` } `; +const inDelayAnimation = keyframes` + 0% { + opacity: 0; + transform: translateY(-5px); + max-height: 0; + } + 50% { + opacity: 0; + transform: translateY(-5px); + max-height: 0; + } + 100% { + opacity: 1; + transform: translateY(0px); + max-height: 6rem; + } +`; + const notificationAnimation = keyframes` 0% { opacity: 0; @@ -117,6 +135,7 @@ export const animations = { expandIn, navbarSlideIn, inAnimation, + inDelayAnimation, outAnimation, notificationAnimation, }; diff --git a/packages/clerk-js/src/ui/utils/test/fixtures.ts b/packages/clerk-js/src/ui/utils/test/fixtures.ts index 7a4ba62f3b0..78902b02af6 100644 --- a/packages/clerk-js/src/ui/utils/test/fixtures.ts +++ b/packages/clerk-js/src/ui/utils/test/fixtures.ts @@ -4,6 +4,7 @@ import type { DisplayConfigJSON, EnvironmentJSON, OrganizationSettingsJSON, + PasswordSettingsData, UserJSON, UserSettingsJSON, } from '@clerk/types'; @@ -154,6 +155,19 @@ const createBaseUserSettings = (): UserSettingsJSON => { socials.map(social => [social, { enabled: false, required: false, authenticatable: false, strategy: social }]), ) as any as UserSettingsJSON['social']; + const passwordSettingsConfig = { + allowed_special_characters: '', + max_length: 0, + min_length: 8, + require_special_char: false, + require_numbers: false, + require_lowercase: false, + require_uppercase: false, + disable_hibp: true, + show_zxcvbn: false, + min_zxcvbn_strength: 0, + } as UserSettingsJSON['password_settings']; + return { attributes: { ...attributeConfig }, actions: { delete_self: false, create_organization: false }, @@ -177,6 +191,7 @@ const createBaseUserSettings = (): UserSettingsJSON => { enabled: false, }, }, + password_settings: passwordSettingsConfig, }; }; diff --git a/packages/clerk-js/src/ui/utils/useFormControl.ts b/packages/clerk-js/src/ui/utils/useFormControl.ts index 3a9c061b581..73f3862943e 100644 --- a/packages/clerk-js/src/ui/utils/useFormControl.ts +++ b/packages/clerk-js/src/ui/utils/useFormControl.ts @@ -2,7 +2,8 @@ import type { ClerkAPIError } from '@clerk/types'; import type { HTMLInputTypeAttribute } from 'react'; import { useState } from 'react'; -import { useSetTimeout } from '../hooks'; +import { DEBOUNCE_MS } from '../../core/constants'; +import { useDebounce } from '../hooks'; import type { LocalizationKey } from '../localization'; import { useLocalizations } from '../localization'; @@ -55,6 +56,7 @@ type FieldStateProps = { setSuccess: (message: string) => void; setInfo: (info: string) => void; setHasPassedComplexity: (b: boolean) => void; + clearFeedback: () => void; hasPassedComplexity: boolean; } & Omit; @@ -64,6 +66,7 @@ export type FormControlState = FieldStateProps & { setInfo: (info: string) => void; setValue: (val: string | undefined) => void; setChecked: (isChecked: boolean) => void; + clearFeedback: () => void; props: FieldStateProps; }; @@ -120,10 +123,14 @@ export const useFormControl = ( const setInfo: FormControlState['setInfo'] = info => { if (info) { - setFeedback({ message: translateError(info), type: 'info' }); + setFeedback({ message: info, type: 'info' }); } }; + const clearFeedback: FormControlState['clearFeedback'] = () => { + setFeedback({ message: '', type: 'info' }); + }; + // eslint-disable-next-line @typescript-eslint/no-unused-vars const { defaultChecked, validatePassword: validatePasswordProp, buildErrorMessage, ...restOpts } = opts; @@ -139,6 +146,7 @@ export const useFormControl = ( feedback: feedback.message || t(opts.infoText), feedbackType: feedback.type, setInfo, + clearFeedback, hasPassedComplexity, setHasPassedComplexity, validatePassword: opts.type === 'password' ? opts.validatePassword : undefined, @@ -174,9 +182,11 @@ type DebouncingOption = { export const useFormControlFeedback = (opts?: DebouncingOption): DebouncedFeedback => { const { feedback = '', delayInMs = 100, feedbackType = 'info', isFocused = false } = opts || {}; - const shouldHide = isFocused ? false : ['info', 'warning'].includes(feedbackType); + const debouncedFocused = useDebounce(isFocused, DEBOUNCE_MS); + + const shouldHide = debouncedFocused ? false : ['info', 'warning'].includes(feedbackType); - const debouncedState = useSetTimeout( + const debouncedState = useDebounce( { feedback: shouldHide ? '' : feedback, feedbackType: shouldHide ? 'info' : feedbackType }, delayInMs, ); From 26ef2423f26f0365524cd7719a6843eceaca383c Mon Sep 17 00:00:00 2001 From: George Desipris Date: Wed, 18 Oct 2023 01:56:18 +0300 Subject: [PATCH 3/3] fix(clerk-js): Wait for zxcvbn before showing success message chore(clerk-js): Replace NodeJS.Timeout type refactor(clerk-js): Extract inAnimation ternary into variable refactor(clerk-js): Handle the undefined case in getElementProps --- .../clerk-js/src/ui/elements/FormControl.tsx | 22 ++++++++++--------- .../src/ui/elements/PasswordInput.tsx | 9 +++++--- packages/clerk-js/src/ui/hooks/useDebounce.ts | 2 +- .../clerk-js/src/ui/utils/useFormControl.ts | 6 ++--- .../clerk-js/src/utils/passwords/password.ts | 11 +++++----- 5 files changed, 27 insertions(+), 23 deletions(-) diff --git a/packages/clerk-js/src/ui/elements/FormControl.tsx b/packages/clerk-js/src/ui/elements/FormControl.tsx index 46b47435eeb..0838dd6e3a3 100644 --- a/packages/clerk-js/src/ui/elements/FormControl.tsx +++ b/packages/clerk-js/src/ui/elements/FormControl.tsx @@ -101,14 +101,13 @@ function useFormTextAnimation() { animation: 'none', }; } + + const inAnimation = options?.inDelay ? animations.inDelayAnimation : animations.inAnimation; + return t => ({ - animation: `${ - enterAnimation - ? options?.inDelay - ? animations.inDelayAnimation - : animations.inAnimation - : animations.outAnimation - } ${t.transitionDuration.$textField} ${t.transitionTiming.$common}`, + animation: `${enterAnimation ? inAnimation : animations.outAnimation} ${t.transitionDuration.$textField} ${ + t.transitionTiming.$common + }`, transition: `height ${t.transitionDuration.$slow} ${t.transitionTiming.$common}`, // This is expensive but required for a smooth layout shift }); }, @@ -205,7 +204,10 @@ export const FormFeedback = (props: FormFeedbackProps) => { return max; }, [heightA, heightB]); - const getElementProps = (type: FormFeedbackDescriptorsKeys) => { + const getElementProps = (type?: FormFeedbackDescriptorsKeys) => { + if (!type) { + return {}; + } const descriptor = (elementDescriptors?.[type] || defaultElementDescriptors[type]) as ElementDescriptor | undefined; return { elementDescriptor: descriptor, @@ -235,7 +237,7 @@ export const FormFeedback = (props: FormFeedbackProps) => { sx={[getFormTextAnimation(!!feedback)]} > ({ @@ -246,7 +248,7 @@ export const FormFeedback = (props: FormFeedbackProps) => { localizationKey={feedbacks.a?.feedback} /> ({ diff --git a/packages/clerk-js/src/ui/elements/PasswordInput.tsx b/packages/clerk-js/src/ui/elements/PasswordInput.tsx index 8f3341f5050..61a2395530c 100644 --- a/packages/clerk-js/src/ui/elements/PasswordInput.tsx +++ b/packages/clerk-js/src/ui/elements/PasswordInput.tsx @@ -21,7 +21,7 @@ export const PasswordInput = forwardRef((p const [hidden, setHidden] = React.useState(true); const { id, onChange: onChangeProp, validatePassword: validatePasswordProp = false, ...rest } = props; const inputRef = useRef(null); - const [timeoutState, setTimeoutState] = useState(null); + const [timeoutState, setTimeoutState] = useState | null>(null); const { userSettings: { passwordSettings }, @@ -41,6 +41,7 @@ export const PasswordInput = forwardRef((p if (inputRef.current === document.activeElement) { formControlProps?.setInfo?.(message); } else { + // Turn the suggestion into an error if not focused. formControlProps?.setError?.(message); } }, @@ -72,11 +73,13 @@ export const PasswordInput = forwardRef((p onChange={onChange} onBlur={e => { rest.onBlur?.(e); - onChange(e); + // Call validate password because to calculate the new feedbackType as the element is now blurred + validatePassword(e.target.value); }} onFocus={e => { rest.onFocus?.(e); - onChange(e); + // Call validate password because to calculate the new feedbackType as the element is now focused + validatePassword(e.target.value); }} //@ts-expect-error ref={mergeRefs(ref, inputRef)} diff --git a/packages/clerk-js/src/ui/hooks/useDebounce.ts b/packages/clerk-js/src/ui/hooks/useDebounce.ts index aea7086d789..b9b28fd66cb 100644 --- a/packages/clerk-js/src/ui/hooks/useDebounce.ts +++ b/packages/clerk-js/src/ui/hooks/useDebounce.ts @@ -2,7 +2,7 @@ import { useEffect, useState } from 'react'; export function useDebounce(value: T, delayInMs?: number): T { const [debouncedValue, setDebouncedValue] = useState(value); - const [timeoutState, setTimeoutState] = useState(undefined); + const [timeoutState, setTimeoutState] = useState | undefined>(undefined); useEffect(() => { const handleDebounce = () => { diff --git a/packages/clerk-js/src/ui/utils/useFormControl.ts b/packages/clerk-js/src/ui/utils/useFormControl.ts index 73f3862943e..8e043f77be8 100644 --- a/packages/clerk-js/src/ui/utils/useFormControl.ts +++ b/packages/clerk-js/src/ui/utils/useFormControl.ts @@ -2,7 +2,6 @@ import type { ClerkAPIError } from '@clerk/types'; import type { HTMLInputTypeAttribute } from 'react'; import { useState } from 'react'; -import { DEBOUNCE_MS } from '../../core/constants'; import { useDebounce } from '../hooks'; import type { LocalizationKey } from '../localization'; import { useLocalizations } from '../localization'; @@ -179,12 +178,11 @@ type DebouncingOption = { isFocused?: boolean; delayInMs?: number; }; + export const useFormControlFeedback = (opts?: DebouncingOption): DebouncedFeedback => { const { feedback = '', delayInMs = 100, feedbackType = 'info', isFocused = false } = opts || {}; - const debouncedFocused = useDebounce(isFocused, DEBOUNCE_MS); - - const shouldHide = debouncedFocused ? false : ['info', 'warning'].includes(feedbackType); + const shouldHide = isFocused ? false : ['info', 'warning'].includes(feedbackType); const debouncedState = useDebounce( { feedback: shouldHide ? '' : feedback, feedbackType: shouldHide ? 'info' : feedbackType }, diff --git a/packages/clerk-js/src/utils/passwords/password.ts b/packages/clerk-js/src/utils/passwords/password.ts index 9abc9e17697..1f6442b0ea6 100644 --- a/packages/clerk-js/src/utils/passwords/password.ts +++ b/packages/clerk-js/src/utils/passwords/password.ts @@ -23,7 +23,7 @@ export const createValidatePassword = (config: UsePasswordConfig, callbacks?: Va const { show_zxcvbn, validatePassword: validatePasswordProp } = config; const getComplexity = createValidateComplexity(config); const getScore = createValidatePasswordStrength(config); - let result = {} satisfies PasswordValidation; + let result: PasswordValidation = {} satisfies PasswordValidation; return (password: string, internalCallbacks?: ValidatePasswordCallbacks) => { const { @@ -65,9 +65,10 @@ export const createValidatePassword = (config: UsePasswordConfig, callbacks?: Va }); } - internalOnValidation({ - ...result, - complexity: failedValidationsComplexity, - }); + if (result.complexity && Object.keys(result.complexity).length === 0 && show_zxcvbn) { + return; + } + + internalOnValidation(result); }; };