From cf3fea0682df5f893111e41e7f3b1b1c96ee849f Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Thu, 25 May 2023 16:55:25 +0100 Subject: [PATCH 01/12] Remove fullShippingAddressPushed action/selectors etc from wc/store/cart --- assets/js/data/cart/actions.ts | 8 -------- assets/js/data/cart/default-state.ts | 1 - assets/js/data/cart/reducers.ts | 9 --------- assets/js/data/cart/resolvers.ts | 5 ----- assets/js/data/cart/selectors.ts | 7 ------- assets/js/types/type-defs/cart.ts | 2 -- 6 files changed, 32 deletions(-) diff --git a/assets/js/data/cart/actions.ts b/assets/js/data/cart/actions.ts index 1f09020e1ff..41b9b8de857 100644 --- a/assets/js/data/cart/actions.ts +++ b/assets/js/data/cart/actions.ts @@ -479,13 +479,6 @@ export const updateCustomerData = } }; -export const setFullShippingAddressPushed = ( - fullShippingAddressPushed: boolean -) => ( { - type: types.SET_FULL_SHIPPING_ADDRESS_PUSHED, - fullShippingAddressPushed, -} ); - type Actions = | typeof addItemToCart | typeof applyCoupon @@ -506,7 +499,6 @@ type Actions = | typeof setShippingAddress | typeof shippingRatesBeingSelected | typeof updateCustomerData - | typeof setFullShippingAddressPushed | typeof updatingCustomerData; export type CartAction = ReturnOrGeneratorYieldUnion< Actions | Thunks >; diff --git a/assets/js/data/cart/default-state.ts b/assets/js/data/cart/default-state.ts index 112f5eeb94f..8ce98387a77 100644 --- a/assets/js/data/cart/default-state.ts +++ b/assets/js/data/cart/default-state.ts @@ -100,7 +100,6 @@ export const defaultCartState: CartState = { applyingCoupon: '', removingCoupon: '', isCartDataStale: false, - fullShippingAddressPushed: false, }, errors: EMPTY_CART_ERRORS, }; diff --git a/assets/js/data/cart/reducers.ts b/assets/js/data/cart/reducers.ts index 3ab809ec8c6..67a9dcfe471 100644 --- a/assets/js/data/cart/reducers.ts +++ b/assets/js/data/cart/reducers.ts @@ -48,15 +48,6 @@ const reducer: Reducer< CartState > = ( action: Partial< CartAction > ) => { switch ( action.type ) { - case types.SET_FULL_SHIPPING_ADDRESS_PUSHED: - state = { - ...state, - metaData: { - ...state.metaData, - fullShippingAddressPushed: action.fullShippingAddressPushed, - }, - }; - break; case types.SET_ERROR_DATA: if ( action.error ) { state = { diff --git a/assets/js/data/cart/resolvers.ts b/assets/js/data/cart/resolvers.ts index 080b021d0db..71b4c386d18 100644 --- a/assets/js/data/cart/resolvers.ts +++ b/assets/js/data/cart/resolvers.ts @@ -9,7 +9,6 @@ import { CartResponse } from '@woocommerce/types'; */ import { CART_API_ERROR } from './constants'; import type { CartDispatchFromMap, CartResolveSelectFromMap } from './index'; -import { shippingAddressHasValidationErrors } from './utils'; /** * Resolver for retrieving all cart data. @@ -28,10 +27,6 @@ export const getCartData = receiveError( CART_API_ERROR ); return; } - - if ( ! shippingAddressHasValidationErrors() ) { - dispatch.setFullShippingAddressPushed( true ); - } receiveCart( cartData ); }; diff --git a/assets/js/data/cart/selectors.ts b/assets/js/data/cart/selectors.ts index 59041eb804e..f4644a4b2d2 100644 --- a/assets/js/data/cart/selectors.ts +++ b/assets/js/data/cart/selectors.ts @@ -222,10 +222,3 @@ export const getItemsPendingQuantityUpdate = ( state: CartState ): string[] => { export const getItemsPendingDelete = ( state: CartState ): string[] => { return state.cartItemsPendingDelete; }; - -/** - * Whether the address has changes that have not been synced with the server. - */ -export const getFullShippingAddressPushed = ( state: CartState ): boolean => { - return state.metaData.fullShippingAddressPushed; -}; diff --git a/assets/js/types/type-defs/cart.ts b/assets/js/types/type-defs/cart.ts index b1fd1cf7b4f..d0ecf0095b8 100644 --- a/assets/js/types/type-defs/cart.ts +++ b/assets/js/types/type-defs/cart.ts @@ -210,8 +210,6 @@ export interface CartMeta { isCartDataStale: boolean; applyingCoupon: string; removingCoupon: string; - /* Whether the full address has been previously pushed to the server */ - fullShippingAddressPushed: boolean; } export interface ExtensionCartUpdateArgs { data: Record< string, unknown >; From f861dd6e9fa0d7e35f950bf3d94fa52903a31600 Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Thu, 25 May 2023 16:57:12 +0100 Subject: [PATCH 02/12] Remove setFullAddressPushed from push-changes --- assets/js/data/cart/push-changes.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/assets/js/data/cart/push-changes.ts b/assets/js/data/cart/push-changes.ts index 784a2b12686..a1cabfa3a23 100644 --- a/assets/js/data/cart/push-changes.ts +++ b/assets/js/data/cart/push-changes.ts @@ -20,7 +20,6 @@ import isShallowEqual from '@wordpress/is-shallow-equal'; import { STORE_KEY } from './constants'; import { VALIDATION_STORE_KEY } from '../validation'; import { processErrorResponse } from '../utils'; -import { shippingAddressHasValidationErrors } from './utils'; type CustomerData = { billingAddress: CartBillingAddress; @@ -212,11 +211,6 @@ const updateCustomerData = debounce( (): void => { ) as BaseAddressKey[] ), ]; } - } ) - .finally( () => { - if ( ! shippingAddressHasValidationErrors() ) { - dispatch( STORE_KEY ).setFullShippingAddressPushed( true ); - } } ); } }, 1000 ); From 3a65366c91c1fac81e3156e5e5350f2624e99204 Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 10:05:48 +0100 Subject: [PATCH 03/12] Change validateInput API to take an object --- .../components/text-input/validated-text-input.tsx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/checkout/components/text-input/validated-text-input.tsx b/packages/checkout/components/text-input/validated-text-input.tsx index 9fcec057f0c..fcae94361d6 100644 --- a/packages/checkout/components/text-input/validated-text-input.tsx +++ b/packages/checkout/components/text-input/validated-text-input.tsx @@ -88,7 +88,7 @@ const ValidatedTextInput = ( { } ); const validateInput = useCallback( - ( errorsHidden = true ) => { + ( { errorsHidden = true, forceRevalidation = false } = {} ) => { const inputObject = inputRef.current || null; if ( inputObject === null ) { @@ -99,7 +99,7 @@ const ValidatedTextInput = ( { inputObject.value = inputObject.value.trim(); inputObject.setCustomValidity( '' ); - if ( previousValue === inputObject.value ) { + if ( previousValue === inputObject.value && ! forceRevalidation ) { return; } @@ -147,7 +147,7 @@ const ValidatedTextInput = ( { inputRef.current !== null && inputRef.current?.ownerDocument?.activeElement !== inputRef.current ) { - validateInput( false ); + validateInput( { errorsHidden: false } ); } // We need to track value even if it is not directly used so we know when it changes. }, [ value, previousValue, validateInput ] ); @@ -170,7 +170,7 @@ const ValidatedTextInput = ( { // if validateOnMount is false, only validate input if focusOnMount is also false if ( validateOnMount || ! focusOnMount ) { - validateInput( true ); + validateInput(); } setIsPristine( false ); @@ -220,13 +220,13 @@ const ValidatedTextInput = ( { hideValidationError( errorIdString ); // Revalidate on user input so we know if the value is valid. - validateInput( true ); + validateInput(); // Push the changes up to the parent component if the value is valid. onChange( val ); } } onBlur={ () => { - validateInput( false ); + validateInput( { errorsHidden: false } ); } } ariaDescribedBy={ describedBy } value={ value } From 82147b558913637118319a479b8572452b86ab65 Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 10:06:46 +0100 Subject: [PATCH 04/12] Add revalidateDependencies to props and prop type definition --- .../checkout/components/text-input/validated-text-input.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/checkout/components/text-input/validated-text-input.tsx b/packages/checkout/components/text-input/validated-text-input.tsx index fcae94361d6..bb1f46cd2da 100644 --- a/packages/checkout/components/text-input/validated-text-input.tsx +++ b/packages/checkout/components/text-input/validated-text-input.tsx @@ -51,6 +51,8 @@ interface ValidatedTextInputProps | undefined; // Whether validation should run when focused - only has an effect when focusOnMount is also true. validateOnMount?: boolean | undefined; + // A set of dependencies to watch, and revalidate if they change. + revalidateDependencies?: unknown[] | undefined; } const ValidatedTextInput = ( { @@ -67,6 +69,7 @@ const ValidatedTextInput = ( { customValidation, label, validateOnMount = true, + revalidateDependencies = [], ...rest }: ValidatedTextInputProps ): JSX.Element => { const [ isPristine, setIsPristine ] = useState( true ); From 4dc8586f24ac326dd5c3fc0313dea27edffae925 Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 10:07:05 +0100 Subject: [PATCH 05/12] Revalidate if any of the revalidateDependencies change --- .../components/text-input/validated-text-input.tsx | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/checkout/components/text-input/validated-text-input.tsx b/packages/checkout/components/text-input/validated-text-input.tsx index bb1f46cd2da..c0d4a7bf25d 100644 --- a/packages/checkout/components/text-input/validated-text-input.tsx +++ b/packages/checkout/components/text-input/validated-text-input.tsx @@ -134,6 +134,15 @@ const ValidatedTextInput = ( { ] ); + useEffect( () => { + if ( isPristine ) { + return; + } + validateInput( { errorsHidden: false, forceRevalidation: true } ); + // Purposely skip running this unless any of the revalidateDependencies change. Also don't run it on mount (isPristine). + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [ ...revalidateDependencies ] ); + /** * Handle browser autofill / changes via data store. * From 7f1f6dcac3bd80c5d1fca53b2e0eee06b1189873 Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 10:07:53 +0100 Subject: [PATCH 06/12] Get country from datastore and use it as revalidationDep on postcode --- .../address-form/address-form.tsx | 26 ++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/assets/js/base/components/cart-checkout/address-form/address-form.tsx b/assets/js/base/components/cart-checkout/address-form/address-form.tsx index 28fb22b0a27..4f7d9f189af 100644 --- a/assets/js/base/components/cart-checkout/address-form/address-form.tsx +++ b/assets/js/base/components/cart-checkout/address-form/address-form.tsx @@ -22,7 +22,7 @@ import { ShippingAddress, } from '@woocommerce/settings'; import { useSelect, useDispatch, dispatch } from '@wordpress/data'; -import { VALIDATION_STORE_KEY } from '@woocommerce/block-data'; +import { CART_STORE_KEY, VALIDATION_STORE_KEY } from '@woocommerce/block-data'; import { FieldValidationStatus } from '@woocommerce/types'; /** @@ -96,11 +96,26 @@ const AddressForm = ( { const { setValidationErrors, clearValidationError } = useDispatch( VALIDATION_STORE_KEY ); - const countryValidationError = useSelect( ( select ) => { - const store = select( VALIDATION_STORE_KEY ); - return store.getValidationError( validationErrorId ); + const { country, countryValidationError } = useSelect( ( select ) => { + const validationStore = select( VALIDATION_STORE_KEY ); + const cartStore = select( CART_STORE_KEY ); + return { + countryValidationError: + validationStore.getValidationError( validationErrorId ), + country: + cartStore.getCartData()?.[ + type === 'shipping' ? 'shippingAddress' : 'billingAddress' + ]?.country, + }; } ); + // This object is used to revalidate the specified fields when any dependency changes. + const revalidateDependencies: { + [ K in keyof AddressFields ]?: unknown[]; + } = { + postcode: [ country ], + }; + const currentFields = useShallowEqual( fields ); const addressFormFields = useMemo( () => { @@ -269,6 +284,9 @@ const AddressForm = ( { label={ field.required ? field.label : field.optionalLabel } + revalidateDependencies={ + revalidateDependencies[ field.key ] + } value={ values[ field.key ] } autoCapitalize={ field.autocapitalize } autoComplete={ field.autocomplete } From fb69305663ce52c0acd2d47ab725026c687c36c6 Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 10:46:55 +0100 Subject: [PATCH 07/12] Add tests for revalidateDependencies --- .../text-input/test/validated-text-input.tsx | 56 +++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/packages/checkout/components/text-input/test/validated-text-input.tsx b/packages/checkout/components/text-input/test/validated-text-input.tsx index e3e081a9cbc..6ba83cbd486 100644 --- a/packages/checkout/components/text-input/test/validated-text-input.tsx +++ b/packages/checkout/components/text-input/test/validated-text-input.tsx @@ -303,5 +303,61 @@ describe( 'ValidatedTextInput', () => { await expect( textInputElement ).toHaveFocus(); await expect( setValidationErrors ).not.toHaveBeenCalled(); } ); + + it( 'revalidates when revalidateDependencies value changes', async () => { + const setValidationErrors = jest.fn(); + wpData.useDispatch.mockImplementation( ( storeName: string ) => { + if ( storeName === VALIDATION_STORE_KEY ) { + return { + ...jest + .requireActual( '@wordpress/data' ) + .useDispatch( storeName ), + setValidationErrors, + }; + } + return jest + .requireActual( '@wordpress/data' ) + .useDispatch( storeName ); + } ); + + const TestComponent = ( { + dependencies, + }: { + dependencies: unknown[]; + } ) => { + const [ inputValue, setInputValue ] = useState( '' ); + return ( + setInputValue( value ) } + value={ inputValue } + label={ 'Test Input' } + required={ true } + customValidation={ ( inputObject ) => { + return inputObject.value === 'Valid Value'; + } } + focusOnMount={ true } + revalidateDependencies={ dependencies } + validateOnMount={ false } + /> + ); + }; + let dependencyToTrack = 'Test'; + const { rerender } = await render( + + ); + await expect( setValidationErrors ).not.toHaveBeenCalled(); + dependencyToTrack = 'Changed'; + await rerender( + + ); + await expect( setValidationErrors ).toHaveBeenCalled(); + dependencyToTrack = 'Changed again'; + await rerender( + + ); + await expect( setValidationErrors ).toHaveBeenCalledTimes( 2 ); + } ); } ); } ); From 36047ab1a7a787e88883a042340a0f64ba4f1151 Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 15:47:14 +0100 Subject: [PATCH 08/12] Only show errors if the value is not empty --- .../checkout/components/text-input/validated-text-input.tsx | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/checkout/components/text-input/validated-text-input.tsx b/packages/checkout/components/text-input/validated-text-input.tsx index c0d4a7bf25d..1c4d7ce8b79 100644 --- a/packages/checkout/components/text-input/validated-text-input.tsx +++ b/packages/checkout/components/text-input/validated-text-input.tsx @@ -138,7 +138,10 @@ const ValidatedTextInput = ( { if ( isPristine ) { return; } - validateInput( { errorsHidden: false, forceRevalidation: true } ); + validateInput( { + errorsHidden: value === '', + forceRevalidation: true, + } ); // Purposely skip running this unless any of the revalidateDependencies change. Also don't run it on mount (isPristine). // eslint-disable-next-line react-hooks/exhaustive-deps }, [ ...revalidateDependencies ] ); From 14d2ba106e5d580d2ebc413b79e38da247448b99 Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 18:23:05 +0100 Subject: [PATCH 09/12] Remove forceValidation prop from validateInput --- .../components/text-input/validated-text-input.tsx | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/checkout/components/text-input/validated-text-input.tsx b/packages/checkout/components/text-input/validated-text-input.tsx index 1c4d7ce8b79..9a7f55e67be 100644 --- a/packages/checkout/components/text-input/validated-text-input.tsx +++ b/packages/checkout/components/text-input/validated-text-input.tsx @@ -91,7 +91,7 @@ const ValidatedTextInput = ( { } ); const validateInput = useCallback( - ( { errorsHidden = true, forceRevalidation = false } = {} ) => { + ( errorsHidden = true ) => { const inputObject = inputRef.current || null; if ( inputObject === null ) { @@ -138,10 +138,7 @@ const ValidatedTextInput = ( { if ( isPristine ) { return; } - validateInput( { - errorsHidden: value === '', - forceRevalidation: true, - } ); + validateInput( value === '' ); // Purposely skip running this unless any of the revalidateDependencies change. Also don't run it on mount (isPristine). // eslint-disable-next-line react-hooks/exhaustive-deps }, [ ...revalidateDependencies ] ); @@ -162,7 +159,7 @@ const ValidatedTextInput = ( { inputRef.current !== null && inputRef.current?.ownerDocument?.activeElement !== inputRef.current ) { - validateInput( { errorsHidden: false } ); + validateInput( false ); } // We need to track value even if it is not directly used so we know when it changes. }, [ value, previousValue, validateInput ] ); From 5bc6343c4f2bd72cf78d53519f059fa4acf683b5 Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 18:23:26 +0100 Subject: [PATCH 10/12] Skip validate on blur if value is unchanged and not required --- .../text-input/validated-text-input.tsx | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/checkout/components/text-input/validated-text-input.tsx b/packages/checkout/components/text-input/validated-text-input.tsx index 9a7f55e67be..eb46b90f474 100644 --- a/packages/checkout/components/text-input/validated-text-input.tsx +++ b/packages/checkout/components/text-input/validated-text-input.tsx @@ -102,10 +102,6 @@ const ValidatedTextInput = ( { inputObject.value = inputObject.value.trim(); inputObject.setCustomValidity( '' ); - if ( previousValue === inputObject.value && ! forceRevalidation ) { - return; - } - const inputIsValid = customValidation ? inputObject.checkValidity() && customValidation( inputObject ) : inputObject.checkValidity(); @@ -238,7 +234,16 @@ const ValidatedTextInput = ( { onChange( val ); } } onBlur={ () => { - validateInput( { errorsHidden: false } ); + // Don't validate on blur if the value is unchanged and the field is not required. + const inputObject = inputRef.current || null; + if ( + inputObject && + inputObject.value === previousValue && + ! inputObject.required + ) { + return; + } + validateInput( false ); } } ariaDescribedBy={ describedBy } value={ value } From 24d7dabcd93115b288c26fb855b73a9cd71f259a Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 18:38:44 +0100 Subject: [PATCH 11/12] Update revalidation on blur logic --- .../checkout/components/text-input/validated-text-input.tsx | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/checkout/components/text-input/validated-text-input.tsx b/packages/checkout/components/text-input/validated-text-input.tsx index eb46b90f474..b38552edc4b 100644 --- a/packages/checkout/components/text-input/validated-text-input.tsx +++ b/packages/checkout/components/text-input/validated-text-input.tsx @@ -236,10 +236,12 @@ const ValidatedTextInput = ( { onBlur={ () => { // Don't validate on blur if the value is unchanged and the field is not required. const inputObject = inputRef.current || null; + if ( inputObject && inputObject.value === previousValue && - ! inputObject.required + ! inputObject.required && + inputObject.value !== '' ) { return; } From 1f7d132f7099e69034d2442db560f1ea8121dbdf Mon Sep 17 00:00:00 2001 From: Thomas Roberts Date: Fri, 26 May 2023 18:39:01 +0100 Subject: [PATCH 12/12] Remove unnecessary dependency. --- packages/checkout/components/text-input/validated-text-input.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/checkout/components/text-input/validated-text-input.tsx b/packages/checkout/components/text-input/validated-text-input.tsx index b38552edc4b..6fbe26152ec 100644 --- a/packages/checkout/components/text-input/validated-text-input.tsx +++ b/packages/checkout/components/text-input/validated-text-input.tsx @@ -121,7 +121,6 @@ const ValidatedTextInput = ( { } ); }, [ - previousValue, clearValidationError, customValidation, errorIdString,