diff --git a/plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/address-form/index.js b/plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/address-form/index.js index 095466f1f7f..3383313ab10 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/address-form/index.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/cart-checkout/address-form/index.js @@ -100,7 +100,8 @@ const AddressForm = ( { } }, [ values, - countryValidationError, + countryValidationError.message, + countryValidationError.hidden, setValidationErrors, clearValidationError, type, diff --git a/plugins/woocommerce-blocks/assets/js/base/components/country-input/stories/index.js b/plugins/woocommerce-blocks/assets/js/base/components/country-input/stories/index.js index d4259d2ac89..1ca3a1c9a7a 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/country-input/stories/index.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/country-input/stories/index.js @@ -27,7 +27,7 @@ const StoryComponent = ( { label, errorMessage } ) => { } = useValidationContext(); useEffect( () => { setValidationErrors( { country: errorMessage } ); - }, [ errorMessage ] ); + }, [ errorMessage, setValidationErrors ] ); const updateCountry = ( country ) => { clearValidationError( 'country' ); selectCountry( country ); diff --git a/plugins/woocommerce-blocks/assets/js/base/components/select/validated.js b/plugins/woocommerce-blocks/assets/js/base/components/select/validated.js index 9f70388d1af..f434ec81bc0 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/select/validated.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/select/validated.js @@ -41,7 +41,7 @@ const ValidatedSelect = ( { clearValidationError, } = useValidationContext(); - const validateSelect = () => { + useEffect( () => { if ( ! required || currentValue ) { clearValidationError( errorId ); } else { @@ -52,18 +52,21 @@ const ValidatedSelect = ( { }, } ); } - }; - - useEffect( () => { - validateSelect(); - }, [ currentValue ] ); + }, [ + clearValidationError, + currentValue, + errorId, + errorMessage, + required, + setValidationErrors, + ] ); // Remove validation errors when unmounted. useEffect( () => { return () => { clearValidationError( errorId ); }; - }, [ errorId ] ); + }, [ clearValidationError, errorId ] ); const error = getValidationError( errorId ) || {}; diff --git a/plugins/woocommerce-blocks/assets/js/base/components/text-input/validated.js b/plugins/woocommerce-blocks/assets/js/base/components/text-input/validated.js index f8a53a6479c..f2a75f49fe8 100644 --- a/plugins/woocommerce-blocks/assets/js/base/components/text-input/validated.js +++ b/plugins/woocommerce-blocks/assets/js/base/components/text-input/validated.js @@ -2,7 +2,7 @@ * External dependencies */ import { __ } from '@wordpress/i18n'; -import { useRef, useEffect } from 'react'; +import { useCallback, useRef, useEffect, useState } from 'react'; import PropTypes from 'prop-types'; import classnames from 'classnames'; import { useValidationContext } from '@woocommerce/base-context'; @@ -27,6 +27,7 @@ const ValidatedTextInput = ( { showError = true, ...rest } ) => { + const [ isPristine, setIsPristine ] = useState( true ); const inputRef = useRef(); const { getValidationError, @@ -39,39 +40,51 @@ const ValidatedTextInput = ( { const textInputId = id || 'textinput-' + instanceId; errorId = errorId || textInputId; - const validateInput = ( errorsHidden = true ) => { - if ( inputRef.current.checkValidity() ) { - clearValidationError( errorId ); - } else { - setValidationErrors( { - [ errorId ]: { - message: - inputRef.current.validationMessage || - __( 'Invalid value.', 'woo-gutenberg-products-block' ), - hidden: errorsHidden, - }, - } ); - } - }; + const validateInput = useCallback( + ( errorsHidden = true ) => { + if ( inputRef.current.checkValidity() ) { + clearValidationError( errorId ); + } else { + setValidationErrors( { + [ errorId ]: { + message: + inputRef.current.validationMessage || + __( + 'Invalid value.', + 'woo-gutenberg-products-block' + ), + hidden: errorsHidden, + }, + } ); + } + }, + [ clearValidationError, errorId, setValidationErrors ] + ); useEffect( () => { - if ( focusOnMount ) { - inputRef.current.focus(); + if ( isPristine ) { + if ( focusOnMount ) { + inputRef.current.focus(); + } + setIsPristine( false ); } - }, [ focusOnMount ] ); + }, [ focusOnMount, isPristine, setIsPristine ] ); useEffect( () => { - if ( validateOnMount ) { - validateInput(); + if ( isPristine ) { + if ( validateOnMount ) { + validateInput(); + } + setIsPristine( false ); } - }, [ validateOnMount ] ); + }, [ isPristine, setIsPristine, validateOnMount, validateInput ] ); // Remove validation errors when unmounted. useEffect( () => { return () => { clearValidationError( errorId ); }; - }, [ errorId ] ); + }, [ clearValidationError, errorId ] ); const errorMessage = getValidationError( errorId ) || {}; const hasError = errorMessage.message && ! errorMessage.hidden; diff --git a/plugins/woocommerce-blocks/assets/js/base/hooks/cart/use-store-cart-item-quantity.js b/plugins/woocommerce-blocks/assets/js/base/hooks/cart/use-store-cart-item-quantity.js index b870f76aa0f..7c2ad96208a 100644 --- a/plugins/woocommerce-blocks/assets/js/base/hooks/cart/use-store-cart-item-quantity.js +++ b/plugins/woocommerce-blocks/assets/js/base/hooks/cart/use-store-cart-item-quantity.js @@ -55,6 +55,7 @@ export const useStoreCartItemQuantity = ( cartItem ) => { }, [ cartItemKey ] ); + const previousIsPendingQuantity = usePrevious( isPendingQuantity ); const isPendingDelete = useSelect( ( select ) => { @@ -67,6 +68,7 @@ export const useStoreCartItemQuantity = ( cartItem ) => { }, [ cartItemKey ] ); + const previousIsPendingDelete = usePrevious( isPendingDelete ); const removeItem = () => { return cartItemKey @@ -78,34 +80,46 @@ export const useStoreCartItemQuantity = ( cartItem ) => { // Observe debounced quantity value, fire action to update server on change. useEffect( () => { - // Don't run it if quantity didn't change but it was set for the first time. - if ( cartItemKey && Number.isFinite( previousDebouncedQuantity ) ) { + if ( + cartItemKey && + Number.isFinite( previousDebouncedQuantity ) && + previousDebouncedQuantity !== debouncedQuantity + ) { changeCartItemQuantity( cartItemKey, debouncedQuantity ).then( triggerFragmentRefresh ); } - }, [ debouncedQuantity, cartItemKey ] ); + }, [ + cartItemKey, + changeCartItemQuantity, + debouncedQuantity, + previousDebouncedQuantity, + ] ); useEffect( () => { - if ( isPendingQuantity ) { - dispatchActions.incrementCalculating(); - } else { - dispatchActions.decrementCalculating(); + if ( previousIsPendingQuantity !== isPendingQuantity ) { + if ( isPendingQuantity ) { + dispatchActions.incrementCalculating(); + } else { + dispatchActions.decrementCalculating(); + } } - }, [ isPendingQuantity ] ); + }, [ dispatchActions, isPendingQuantity, previousIsPendingQuantity ] ); useEffect( () => { - if ( isPendingDelete ) { - dispatchActions.incrementCalculating(); - } else if ( ! isPendingDelete && cartErrors.length ) { - dispatchActions.decrementCalculating(); + if ( previousIsPendingDelete !== isPendingDelete ) { + if ( isPendingDelete ) { + dispatchActions.incrementCalculating(); + } else { + dispatchActions.decrementCalculating(); + } } return () => { if ( isPendingDelete ) { dispatchActions.decrementCalculating(); } }; - }, [ isPendingDelete ] ); + }, [ dispatchActions, isPendingDelete, previousIsPendingDelete ] ); return { isPendingDelete, diff --git a/plugins/woocommerce-blocks/assets/js/blocks/cart-checkout/cart/full-cart/index.js b/plugins/woocommerce-blocks/assets/js/blocks/cart-checkout/cart/full-cart/index.js index 6f9dcbad78a..fbc2e162fe5 100644 --- a/plugins/woocommerce-blocks/assets/js/blocks/cart-checkout/cart/full-cart/index.js +++ b/plugins/woocommerce-blocks/assets/js/blocks/cart-checkout/cart/full-cart/index.js @@ -85,7 +85,7 @@ const Cart = ( { attributes } ) => { id: error.code, } ); } ); - }, [ cartItemErrors ] ); + }, [ addErrorNotice, cartItemErrors ] ); const totalsCurrency = getCurrencyFromPriceResponse( cartTotals ); diff --git a/plugins/woocommerce-blocks/assets/js/blocks/cart-checkout/cart/test/block.js b/plugins/woocommerce-blocks/assets/js/blocks/cart-checkout/cart/test/block.js index 3b9819fb38b..2e9370484ad 100644 --- a/plugins/woocommerce-blocks/assets/js/blocks/cart-checkout/cart/test/block.js +++ b/plugins/woocommerce-blocks/assets/js/blocks/cart-checkout/cart/test/block.js @@ -40,13 +40,7 @@ describe( 'Testing cart', () => { screen.getByText( /Proceed to Checkout/i ) ).toBeInTheDocument(); - /** - * @todo Investigate extra POST request on initial cart render. - * - * We have an unfixed bug in our test in which the full cart triggers a POST - * request to `wc/store/cart/update-item` causing the fetch to be called twice. - */ - expect( fetchMock ).toHaveBeenCalledTimes( 2 ); + expect( fetchMock ).toHaveBeenCalledTimes( 1 ); } ); it( 'renders empty cart if there are no items in the cart', async () => { fetchMock.mockResponse( ( req ) => { diff --git a/plugins/woocommerce-blocks/assets/js/hocs/with-attributes.js b/plugins/woocommerce-blocks/assets/js/hocs/with-attributes.js index 96820808c20..f7a2e9f7c1e 100644 --- a/plugins/woocommerce-blocks/assets/js/hocs/with-attributes.js +++ b/plugins/woocommerce-blocks/assets/js/hocs/with-attributes.js @@ -3,7 +3,6 @@ */ import { useState, useEffect } from '@wordpress/element'; import { getAttributes, getTerms } from '@woocommerce/editor-components/utils'; -import { find } from 'lodash'; /** * Internal dependencies @@ -14,12 +13,12 @@ import { formatError } from '../base/utils/errors.js'; * Get attribute data (name, taxonomy etc) from server data. * * @param {number} attributeId Attribute ID to look for. - * @param {Array} attributeList List of attributes. + * @param {Array|null} attributeList List of attributes. * @param {string} matchField Field to match on. e.g. id or slug. */ const getAttributeData = ( attributeId, attributeList, matchField = 'id' ) => { - return attributeList - ? find( attributeList, [ matchField, attributeId ] ) + return Array.isArray( attributeList ) + ? attributeList.find( ( attr ) => attr[ matchField ] === attributeId ) : null; }; @@ -30,8 +29,9 @@ const getAttributeData = ( attributeId, attributeList, matchField = 'id' ) => { */ const withAttributes = ( OriginalComponent ) => { return ( props ) => { - const { selected } = props; - const [ attributes, setAttributes ] = useState( [] ); + const { selected = [] } = props; + const selectedSlug = selected.length ? selected[ 0 ].attr_slug : null; + const [ attributes, setAttributes ] = useState( null ); const [ expandedAttribute, setExpandedAttribute ] = useState( 0 ); const [ termsList, setTermsList ] = useState( {} ); const [ loading, setLoading ] = useState( true ); @@ -39,43 +39,44 @@ const withAttributes = ( OriginalComponent ) => { const [ error, setError ] = useState( null ); useEffect( () => { - getAttributes() - .then( ( newAttributes ) => { - newAttributes = newAttributes.map( ( attribute ) => ( { - ...attribute, - parent: 0, - } ) ); + if ( attributes === null ) { + getAttributes() + .then( ( newAttributes ) => { + newAttributes = newAttributes.map( ( attribute ) => ( { + ...attribute, + parent: 0, + } ) ); - setAttributes( newAttributes ); + setAttributes( newAttributes ); - if ( selected.length > 0 ) { - const selectedAttributeFromTerm = attributes - ? getAttributeData( - selected[ 0 ].attr_slug, - newAttributes, - 'taxonomy' - ) - : null; - - if ( selectedAttributeFromTerm ) { - setExpandedAttribute( - selectedAttributeFromTerm.id + if ( selectedSlug ) { + const selectedAttributeFromTerm = getAttributeData( + selectedSlug, + newAttributes, + 'taxonomy' ); + + if ( selectedAttributeFromTerm ) { + setExpandedAttribute( + selectedAttributeFromTerm.id + ); + } } - } - } ) - .catch( async ( e ) => { - setError( await formatError( e ) ); - } ) - .finally( () => { - setLoading( false ); - } ); - }, [] ); + } ) + .catch( async ( e ) => { + setError( await formatError( e ) ); + } ) + .finally( () => { + setLoading( false ); + } ); + } + }, [ attributes, selectedSlug ] ); useEffect( () => { - const attributeData = attributes - ? getAttributeData( expandedAttribute, attributes ) - : null; + const attributeData = getAttributeData( + expandedAttribute, + attributes + ); if ( ! attributeData ) { return; @@ -91,10 +92,10 @@ const withAttributes = ( OriginalComponent ) => { attr_slug: attributeData.taxonomy, } ) ); - setTermsList( { - ...termsList, + setTermsList( ( previousTermsList ) => ( { + ...previousTermsList, [ expandedAttribute ]: newTerms, - } ); + } ) ); } ) .catch( async ( e ) => { setError( await formatError( e ) ); @@ -107,7 +108,7 @@ const withAttributes = ( OriginalComponent ) => { return ( { ? { showIcon: isActive } : {}; return { - ...options, + ...prevOptions, style: { - ...options.style, + ...prevOptions.style, base: { - ...options.style.base, + ...prevOptions.style.base, '::placeholder': { color, },