Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Add revalidateDependencies to ValidatedTextInput #9611

Closed
wants to merge 12 commits into from

Conversation

opr
Copy link
Contributor

@opr opr commented May 26, 2023

This is based on fix/address-pushing and includes changes from #9606.

This PR introduces a new property on ValidatedTextInput called revalidateDependencies which is intended to work the same as useEffect's dependencies array. This prop will be an array of variables. When one of the variables changes, it will call validateInput again.

I added an object in address-form.tsx which is keyed by existing fields, and the values are arrays of dependencies that should cause the field to revalidated. When rendering a field, the object is checked and if there's an entry for that field, the value is passed to ValidateInput's revalidateDependencies prop. The only items in there now are: postcode: [ country ] which means that when rendering the postcode field, changing the country will cause it to revalidate. (The country is derived from the wc/store/cart data store in the useSelect above).

Finally, I moved the if ( previousValue === inputObject.value ) { check to the onBlur function of ValidatedTextInput this is so that when the coupon code shows an error, it does not clear when blurring the field.

Fixes #9591

Testing

Automated Tests

  • Changes in this PR are covered by Automated Tests.
    • Unit tests
    • E2E tests

User Facing Testing

  1. In an incognito window...
  2. Add an item to your cart, go to the Checkout block.
  3. Click the "Apply a coupon" link.
  4. Enter an invalid coupon, and see the error. Click into and out of the field a few times, ensure the error remains.
  5. Type a new code into the box (no need to submit) and ensure the error goes away.
  6. Click into the First name field, and then out of it. Ensure an error shows. Repeat for last name.
  7. Enter a full address using United Kingdom as the country, enter L1 0BP as the Postcode.
  8. Change country to Portugal. Do not change anything else.
  9. Ensure the postcode field is marked as invalid.
  10. Click into the country field, do not type in the box - scroll down to United Kingdom. Select it.
  11. Ensure the postcode error is removed.
  12. Go back to Portugal, ensure the error reappears.
  13. Try to check out, ensure checkout cannot be completed.
  14. Enter 8200-294 into the postcode box and check out successfully.
  • Do not include in the Testing Notes

WooCommerce Visibility

  • WooCommerce Core
  • Feature plugin
  • Experimental

Performance Impact

Changelog

Revalidate the postcode when changing country in the Checkout form.

@opr opr added status: needs review type: bug The issue/PR concerns a confirmed bug. block: checkout Issues related to the checkout block. labels May 26, 2023
@opr opr self-assigned this May 26, 2023
@woocommercebot woocommercebot requested review from a team and tarhi-saad and removed request for a team May 26, 2023 14:58
@github-actions
Copy link
Contributor

github-actions bot commented May 26, 2023

The release ZIP for this PR is accessible via:

https://wcblocks.wpcomstaging.com/wp-content/uploads/woocommerce-gutenberg-products-block-9611.zip

Script Dependencies Report

The compare-assets action has detected some changed script dependencies between this branch and trunk. Please review and confirm the following are correct before merging.

Script Handle Added Removed
reviews-frontend.js wc-settings, wp-a11y, wp-api-fetch, wp-compose, wp-element, wp-i18n, wp-is-shallow-equal, wp-polyfill ⚠️
active-filters-frontend.js wc-blocks-data-store, wc-price-format, wc-settings, wp-data, wp-element, wp-html-entities, wp-i18n, wp-is-shallow-equal, wp-polyfill, wp-primitives, wp-url ⚠️
all-products-frontend.js lodash, react, wc-blocks-checkout, wc-blocks-data-store, wc-blocks-registry, wc-blocks-shared-context, wc-blocks-shared-hocs, wc-price-format, wc-settings, wp-a11y, wp-api-fetch, wp-autop, wp-blocks, wp-compose, wp-data, wp-deprecated, wp-dom, wp-element, wp-hooks, wp-html-entities, wp-i18n, wp-is-shallow-equal, wp-polyfill, wp-primitives, wp-style-engine, wp-url, wp-warning, wp-wordcount ⚠️
cart-frontend.js lodash, react, wc-blocks-checkout, wc-blocks-data-store, wc-blocks-registry, wc-blocks-shared-context, wc-blocks-shared-hocs, wc-price-format, wc-settings, wp-a11y, wp-api-fetch, wp-autop, wp-blocks, wp-compose, wp-data, wp-deprecated, wp-dom, wp-element, wp-hooks, wp-html-entities, wp-i18n, wp-is-shallow-equal, wp-keycodes, wp-plugins, wp-polyfill, wp-primitives, wp-style-engine, wp-url, wp-warning, wp-wordcount ⚠️
checkout-frontend.js lodash, react, wc-blocks-checkout, wc-blocks-data-store, wc-blocks-registry, wc-blocks-shared-hocs, wc-price-format, wc-settings, wp-a11y, wp-api-fetch, wp-autop, wp-compose, wp-data, wp-deprecated, wp-dom, wp-element, wp-hooks, wp-html-entities, wp-i18n, wp-is-shallow-equal, wp-keycodes, wp-plugins, wp-polyfill, wp-primitives, wp-url, wp-warning, wp-wordcount ⚠️
filter-wrapper-frontend.js lodash, react, wc-blocks-checkout, wc-blocks-data-store, wc-blocks-registry, wc-price-format, wc-settings, wp-a11y, wp-compose, wp-data, wp-deprecated, wp-dom, wp-element, wp-html-entities, wp-i18n, wp-is-shallow-equal, wp-keycodes, wp-polyfill, wp-primitives, wp-style-engine, wp-url, wp-warning ⚠️
mini-cart-frontend.js wc-price-format, wp-i18n, wp-polyfill ⚠️
rating-filter-frontend.js lodash, react, wc-blocks-checkout, wc-blocks-data-store, wc-settings, wp-a11y, wp-compose, wp-data, wp-deprecated, wp-dom, wp-element, wp-i18n, wp-is-shallow-equal, wp-keycodes, wp-polyfill, wp-primitives, wp-url, wp-warning ⚠️
mini-cart-component-frontend.js lodash, react, wc-blocks-checkout, wc-blocks-data-store, wc-blocks-registry, wc-price-format, wc-settings, wp-a11y, wp-autop, wp-compose, wp-data, wp-deprecated, wp-dom, wp-element, wp-hooks, wp-html-entities, wp-i18n, wp-is-shallow-equal, wp-keycodes, wp-polyfill, wp-primitives, wp-style-engine, wp-url, wp-warning, wp-wordcount ⚠️

This comment was automatically generated by the ./github/compare-assets action.

TypeScript Errors Report

  • Files with errors: 472
  • Total errors: 2264

⚠️ ⚠️ This PR introduces new TS errors on 4 files:

assets/js/blocks/product-collection/edit.tsx

assets/js/blocks/product-top-rated/block.js

assets/js/blocks/product-top-rated/index.js

packages/checkout/components/text-input/test/validated-text-input.tsx

comments-aggregator

@github-actions
Copy link
Contributor

github-actions bot commented May 26, 2023

Size Change: +578 B (0%)

Total Size: 1.09 MB

Filename Size Change
build/blocks-checkout.js 35.2 kB +41 B (0%)
build/cart-blocks/order-summary-shipping-frontend.js 17.1 kB +107 B (+1%)
build/cart-frontend.js 29.7 kB -1 B (0%)
build/cart.js 45.1 kB +108 B (0%)
build/checkout-blocks/billing-address--checkout-blocks/shipping-address-frontend.js 4.81 kB +106 B (+2%)
build/checkout-blocks/order-summary-shipping-frontend.js 17.2 kB +108 B (+1%)
build/checkout-frontend.js 31.3 kB +2 B (0%)
build/checkout.js 46.4 kB +107 B (0%)
ℹ️ View Unchanged
Filename Size
build/active-filters-frontend.js 8.57 kB
build/active-filters-wrapper-frontend.js 7.62 kB
build/active-filters.js 7.47 kB
build/all-products-frontend.js 11.9 kB
build/all-products.js 39.9 kB
build/all-reviews.js 7.77 kB
build/attribute-filter-wrapper--stock-filter-wrapper-frontend.js 4.04 kB
build/attribute-filter-wrapper-frontend.js 4.28 kB
build/attribute-filter.js 13.1 kB
build/breadcrumbs.js 2.04 kB
build/cart-blocks/cart-accepted-payment-methods-frontend.js 1.39 kB
build/cart-blocks/cart-cross-sells-frontend.js 253 B
build/cart-blocks/cart-cross-sells-products--product-price-frontend.js 2.94 kB
build/cart-blocks/cart-cross-sells-products-frontend.js 3.73 kB
build/cart-blocks/cart-express-payment--checkout-blocks/express-payment-frontend.js 5.17 kB
build/cart-blocks/cart-express-payment-frontend.js 718 B
build/cart-blocks/cart-items-frontend.js 301 B
build/cart-blocks/cart-line-items--mini-cart-contents-block/products-table-frontend.js 5.51 kB
build/cart-blocks/cart-line-items-frontend.js 1.06 kB
build/cart-blocks/cart-order-summary-frontend.js 1.28 kB
build/cart-blocks/cart-totals-frontend.js 308 B
build/cart-blocks/empty-cart-frontend.js 346 B
build/cart-blocks/filled-cart-frontend.js 656 B
build/cart-blocks/order-summary-coupon-form-frontend.js 1.63 kB
build/cart-blocks/order-summary-discount-frontend.js 2.13 kB
build/cart-blocks/order-summary-fee-frontend.js 272 B
build/cart-blocks/order-summary-heading-frontend.js 333 B
build/cart-blocks/order-summary-subtotal-frontend.js 273 B
build/cart-blocks/order-summary-taxes-frontend.js 434 B
build/cart-blocks/proceed-to-checkout-frontend.js 1.38 kB
build/catalog-sorting.js 1.7 kB
build/checkout-blocks/actions-frontend.js 1.85 kB
build/checkout-blocks/billing-address-frontend.js 1.18 kB
build/checkout-blocks/contact-information-frontend.js 2.05 kB
build/checkout-blocks/express-payment-frontend.js 1.13 kB
build/checkout-blocks/fields-frontend.js 330 B
build/checkout-blocks/order-note-frontend.js 1.14 kB
build/checkout-blocks/order-summary-cart-items-frontend.js 3.69 kB
build/checkout-blocks/order-summary-coupon-form-frontend.js 1.79 kB
build/checkout-blocks/order-summary-discount-frontend.js 2.29 kB
build/checkout-blocks/order-summary-fee-frontend.js 275 B
build/checkout-blocks/order-summary-frontend.js 1.28 kB
build/checkout-blocks/order-summary-subtotal-frontend.js 273 B
build/checkout-blocks/order-summary-taxes-frontend.js 435 B
build/checkout-blocks/payment-frontend.js 8.29 kB
build/checkout-blocks/pickup-options-frontend.js 4.8 kB
build/checkout-blocks/shipping-address-frontend.js 1.17 kB
build/checkout-blocks/shipping-method-frontend.js 2.59 kB
build/checkout-blocks/shipping-methods-frontend.js 6.35 kB
build/checkout-blocks/terms-frontend.js 1.56 kB
build/checkout-blocks/totals-frontend.js 312 B
build/customer-account.js 3.18 kB
build/featured-category.js 15 kB
build/featured-product.js 15.2 kB
build/filter-wrapper-frontend.js 14.2 kB
build/filter-wrapper.js 2.39 kB
build/general-style-rtl.css 1.31 kB
build/general-style.css 1.31 kB
build/handpicked-products.js 8 kB
build/legacy-template.js 6.45 kB
build/mini-cart-component-frontend.js 28.4 kB
build/mini-cart-contents-block/cart-button-frontend.js 1.73 kB
build/mini-cart-contents-block/checkout-button-frontend.js 1.74 kB
build/mini-cart-contents-block/empty-cart-frontend.js 360 B
build/mini-cart-contents-block/filled-cart-frontend.js 267 B
build/mini-cart-contents-block/footer-frontend.js 4.1 kB
build/mini-cart-contents-block/items-frontend.js 237 B
build/mini-cart-contents-block/products-table-frontend.js 592 B
build/mini-cart-contents-block/shopping-button-frontend.js 528 B
build/mini-cart-contents-block/title-frontend.js 1.91 kB
build/mini-cart-contents-block/title-items-counter-frontend.js 1.6 kB
build/mini-cart-contents-block/title-label-frontend.js 1.54 kB
build/mini-cart-contents.js 18 kB
build/mini-cart-frontend.js 2.66 kB
build/mini-cart.js 4.35 kB
build/price-filter-wrapper-frontend.js 6.75 kB
build/price-filter.js 8.47 kB
build/price-format.js 1.19 kB
build/product-add-to-cart--product-button--product-image--product-price--product-rating--product-sale-bad--49d3ecb2.js 251 B
build/product-add-to-cart--product-button--product-image--product-rating--product-title.js 151 B
build/product-add-to-cart-frontend.js 6.51 kB
build/product-add-to-cart.js 8.87 kB
build/product-best-sellers.js 8.34 kB
build/product-button--product-image--product-price--product-rating--product-sale-badge--product-sku--prod--5bce0384.js 963 B
build/product-button-frontend.js 2.65 kB
build/product-button.js 4.01 kB
build/product-categories.js 2.37 kB
build/product-category.js 9.35 kB
build/product-collection.js 3.39 kB
build/product-image-frontend.js 2.63 kB
build/product-image.js 4.18 kB
build/product-new.js 8.34 kB
build/product-on-sale.js 8.68 kB
build/product-price-frontend.js 203 B
build/product-price.js 1.68 kB
build/product-query.js 11.7 kB
build/product-rating-frontend.js 2.31 kB
build/product-rating.js 999 B
build/product-results-count.js 1.66 kB
build/product-sale-badge-frontend.js 1.8 kB
build/product-sale-badge.js 666 B
build/product-search.js 2.63 kB
build/product-sku-frontend.js 1.84 kB
build/product-sku.js 535 B
build/product-stock-indicator-frontend.js 2.02 kB
build/product-stock-indicator.js 731 B
build/product-summary-frontend.js 2.19 kB
build/product-summary.js 904 B
build/product-tag.js 8.97 kB
build/product-template.js 3.27 kB
build/product-title-frontend.js 2.22 kB
build/product-title.js 3.69 kB
build/product-top-rated.js 8.58 kB
build/products-by-attribute.js 9.7 kB
build/rating-filter-frontend.js 21.3 kB
build/rating-filter-wrapper-frontend.js 6.21 kB
build/rating-filter.js 6.89 kB
build/reviews-by-category.js 12.1 kB
build/reviews-by-product.js 13.2 kB
build/reviews-frontend.js 7.11 kB
build/single-product.js 11.1 kB
build/stock-filter-wrapper-frontend.js 2.98 kB
build/stock-filter.js 7.61 kB
build/store-notices.js 1.69 kB
build/vendors--attribute-filter-wrapper--cart-blocks/order-summary-coupon-form--cart-blocks/order-summary--48e1e4bb-frontend.js 6.82 kB
build/vendors--attribute-filter-wrapper--cart-blocks/order-summary-shipping--checkout-blocks/billing-addr--d9f38f9d-frontend.js 4.21 kB
build/vendors--attribute-filter-wrapper--stock-filter-wrapper-frontend.js 5.11 kB
build/vendors--cart-blocks/cart-cross-sells-products--cart-blocks/cart-line-items--cart-blocks/cart-order--3c5fe802-frontend.js 5.26 kB
build/vendors--cart-blocks/cart-line-items--checkout-blocks/order-summary-cart-items--mini-cart-contents---233ab542-frontend.js 3.57 kB
build/vendors--cart-blocks/order-summary-shipping--checkout-blocks/billing-address--checkout-blocks/order--decc3dc6-frontend.js 19.4 kB
build/vendors--checkout-blocks/pickup-options--checkout-blocks/shipping-methods-frontend.js 8.25 kB
build/vendors--checkout-blocks/shipping-method-frontend.js 12.5 kB
build/vendors--price-filter-wrapper-frontend.js 2.2 kB
build/vendors--product-add-to-cart-frontend.js 7.26 kB
build/vendors--rating-filter-wrapper-frontend.js 5.11 kB
build/wc-blocks-data.js 22.3 kB
build/wc-blocks-editor-style-rtl.css 5.96 kB
build/wc-blocks-editor-style.css 5.96 kB
build/wc-blocks-google-analytics.js 1.56 kB
build/wc-blocks-middleware.js 933 B
build/wc-blocks-registry.js 3.15 kB
build/wc-blocks-shared-context.js 1.52 kB
build/wc-blocks-shared-hocs.js 1.75 kB
build/wc-blocks-style-rtl.css 27.8 kB
build/wc-blocks-style.css 27.8 kB
build/wc-blocks-vendors-style-rtl.css 1.96 kB
build/wc-blocks-vendors-style.css 1.96 kB
build/wc-blocks-vendors.js 65 kB
build/wc-blocks.js 3.7 kB
build/wc-payment-method-bacs.js 816 B
build/wc-payment-method-cheque.js 811 B
build/wc-payment-method-cod.js 909 B
build/wc-payment-method-paypal.js 837 B
build/wc-settings.js 2.6 kB
build/wc-shipping-method-pickup-location.js 30.3 kB
build/woo-directives-runtime.js 2.73 kB
build/woo-directives-vendors.js 7.91 kB

compressed-size-action

@ralucaStan
Copy link
Contributor

Previously, the validation would not run if the previous and current values were the same, however in cases of revalidation we want it to.

This check of previous and current value

if ( previousValue === inputObject.value ) {
was introduced in #8349 as a way to fix the Coupon field bug, but I noticed (https://github.com/woocommerce/woocommerce-blocks/compare/try/debug-address-field-validation?expand=1) it also impacted use-cases when the validation should have happened on the same value was still present in the input. See #9377

I wonder if there isn't another way of fixing the Coupon code instead of introducing this new field in the ValidatedTextInput API.

@opr
Copy link
Contributor Author

opr commented May 26, 2023

Previously, the validation would not run if the previous and current values were the same, however in cases of revalidation we want it to.

This check of previous and current value

if ( previousValue === inputObject.value ) {

was introduced in #8349 as a way to fix the Coupon field bug, but I noticed (https://github.com/woocommerce/woocommerce-blocks/compare/try/debug-address-field-validation?expand=1) it also impacted use-cases when the validation should have happened on the same value was still present in the input. See #9377
I wonder if there isn't another way of fixing the Coupon code instead of introducing this new field in the ValidatedTextInput API.

Thanks for the comment! Maybe I'm missing something but I don't see how the check for if ( previousValue === inputObject.value ) { means #9377 is required?

I believe this new prop is required because we want to revalidate something based on a factor external to the input field. The useEffects in ValidatedTextInput rely on the input's value changing, and the only way to trigger those is to change one of their dependencies.

I actually moved if ( previousValue === inputObject.value ) { into the onBlur that way the extra forceValidation prop on validateInput is not needed, and the coupon input does not clear its error when blurred.

I have updated the testing instructions and PR body.

Copy link
Contributor

@tarhi-saad tarhi-saad left a comment

Choose a reason for hiding this comment

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

Thank you for working on this PR, @opr! LGTM! 👍

@ralucaStan
Copy link
Contributor

ralucaStan commented May 30, 2023

Thanks for the comment! Maybe I'm missing something but I don't see how the check for if ( previousValue === inputObject.value ) { means #9377 is required?

I don't understand this question :)
My point was that that check was one of the reasons causing #9377. Entering with the same value and a different required value would return early in the validation function because of this check, even if the field needed to be revalidated.

I believe this new prop is required because we want to revalidate something based on a factor external to the input field. The useEffects in ValidatedTextInput rely on the input's value changing, and the only way to trigger those is to change one of their dependencies.

#9377 actually reveals that we need to revalidate if the required property changes, even if the value of the input remains the same.
My concern with this approach is that it's imperative. We will for sure add other field relationships there as the locale fields are related.

So I was proposing to fix this issue with another approach, by removing the same value check. I am curious about what performance benefits we gain for skipping validation for fields that have the same value and if it's worth it to include another level of logic in our API.

Later edit: If we need to re-trigger validation it should be done at form level, not at a specific field's level. This is an API we can also offer to extensions who'd want to retrigger validation after a certain action.

@opr opr marked this pull request as draft May 30, 2023 11:28
Base automatically changed from fix/address-pushing to trunk May 30, 2023 15:46
@opr
Copy link
Contributor Author

opr commented May 30, 2023

closed in favour of a more general approach - issue to track this more general approach

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
block: checkout Issues related to the checkout block. type: bug The issue/PR concerns a confirmed bug.
Projects
None yet
3 participants