-
Notifications
You must be signed in to change notification settings - Fork 221
Add clearValidationErrors
action to validation data store
#7601
Conversation
The release ZIP for this PR is accessible via:
|
TypeScript Errors ReportFiles with errors: 431 🎉 🎉 This PR does not introduce new TS errors. |
Size Change: +115 B (0%) Total Size: 991 kB
ℹ️ View Unchanged
|
da9e748
to
dd1db2e
Compare
TypeScript Errors ReportFiles with errors: 445 🎉 🎉 This PR does not introduce new TS errors. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, just a suggestion and a bit of confusion in the test, see beiow 👍
export type ValidationAction = ReturnOrGeneratorYieldUnion< | ||
| typeof setValidationErrors | ||
| typeof clearAllValidationErrors | ||
| typeof clearValidationError | ||
| typeof clearValidationErrors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about combining clearAllValidationErrors
and clearValidationErrors
so that the default of clearValidationErrors
if you don't specify an argument is to clear all the errors?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice idea actually. We should deprecate clearAllValidationErrors
if we go ahead with this (instead of removing it) since it's currently out in the wild
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good point. I think it'll neaten things up. Do you want to do it as part of this PR? What's the process for deprecating things?
fb4961a
to
4eabac0
Compare
TypeScript Errors ReportFiles with errors: 446 🎉 🎉 This PR does not introduce new TS errors. |
1 similar comment
TypeScript Errors ReportFiles with errors: 446 🎉 🎉 This PR does not introduce new TS errors. |
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now - just one minor thing (see below)
#### _Parameters_ | ||
|
||
- _errors_ `string[] | undefined` - The error IDs to clear validation errors for. This can be undefined, and if it | ||
is, all validation errors will be cleared. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something funky going on with the spacing here. Can you check prettier runs on this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one thanks Alex, I've updated that now.
fb4d2aa
to
9042521
Compare
TypeScript Errors ReportFiles with errors: 446
assets/js/atomic/blocks/product-elements/image/test/block.test.js
assets/js/atomic/blocks/product-elements/price/index.js assets/js/atomic/blocks/product-elements/stock-indicator/block.js assets/js/atomic/blocks/product-elements/stock-indicator/edit.js assets/js/atomic/blocks/product-elements/tag-list/index.ts assets/js/base/components/cart-checkout/address-form/address-form.tsx assets/js/base/components/cart-checkout/product-details/index.tsx assets/js/base/components/cart-checkout/totals/coupon/index.tsx assets/js/base/components/product-price/index.tsx assets/js/base/components/state-input/state-input.tsx assets/js/base/components/text-input/text-input.tsx assets/js/base/components/text-input/validated-text-input.tsx assets/js/base/context/hooks/use-checkout-submit.js assets/js/blocks/cart/block.js assets/js/blocks/checkout/block.tsx assets/js/blocks/checkout/inner-blocks/checkout-contact-information-block/block.tsx assets/js/blocks/checkout/phone-number/index.tsx assets/js/blocks/mini-cart/frontend.ts assets/js/blocks/product-query/constants.ts assets/js/data/payment/check-payment-methods.ts assets/js/types/type-defs/payment-method-interface.ts packages/checkout/blocks-registry/get-registered-blocks.ts packages/checkout/blocks-registry/index.ts packages/checkout/blocks-registry/register-checkout-block.ts packages/checkout/components/index.js packages/checkout/components/order-meta/index.js packages/checkout/components/totals/index.js packages/checkout/index.js packages/checkout/slot/index.js packages/checkout/utils/index.js |
This PR allows multiple validation errors to be cleared in a single action. In cases where clearing multiple errors at once is required, this would bring a slight performance increase by reducing the number of actions dispatched onto the store.
Testing
Automated Tests
Internal dev testing

2. Open the console and enterUser Facing Testing
These tests are for regression testing only. It does not test the new feature.
WooCommerce Visibility
Performance Impact
Changelog