-
Notifications
You must be signed in to change notification settings - Fork 219
New contexts for StoreNoticesContainer
and notice grouping
#7711
Conversation
The release ZIP for this PR is accessible via:
|
TypeScript Errors ReportFiles with errors: 439
assets/js/base/components/cart-checkout/address-form/address-form.tsx
assets/js/base/context/providers/cart-checkout/checkout-events/index.tsx assets/js/data/cart/resolvers.ts assets/js/data/checkout/types.ts assets/js/data/checkout/utils.ts assets/js/data/index.ts assets/js/data/payment/types.ts assets/js/data/shared-controls.ts assets/js/data/validation/actions.ts assets/js/data/validation/reducers.ts assets/js/data/validation/test/reducers.ts assets/js/data/validation/test/selectors.ts packages/checkout/components/store-notices-container/snackbar-notices.tsx packages/checkout/components/store-notices-container/store-notices.tsx packages/checkout/components/store-notices-container/test/index.tsx |
Size Change: -3.61 kB (0%) Total Size: 1 MB
ℹ️ View Unchanged
|
Script Dependencies ReportThe
This comment was automatically generated by the |
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 Mike, this tested well, but I think we do need to address the refs being in the data store.
We should also deprecate the snackbarNoticeVisibility
filter properly imo.
Please see my comments below and let me know if anything is unclear or needs further testing.
packages/checkout/components/store-notices-container/snackbar-notices.tsx
Outdated
Show resolved
Hide resolved
packages/checkout/components/store-notices-container/store-notices.tsx
Outdated
Show resolved
Hide resolved
assets/js/blocks/checkout/inner-blocks/checkout-contact-information-block/block.tsx
Outdated
Show resolved
Hide resolved
@opr I've worked on everything you mentioned. I'll remove the test buttons before merge, and log/patch points and rewards for the new action name. |
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 work, thanks Mike.
I tested it using MMQ and modifying the stock status of various items in my cart.
I am wondering if we should add some testing instructions for Globalstep/release lead for when this gets merged?
@opr Compatibility patch: https://github.com/woocommerce/woocommerce-points-and-rewards/pull/491 There was a |
@opr By modifying cart items quantities? We can also trigger a snackbar with an invalid postcode, but we should be fixing that soon I think. |
Yep, while on the Checkout page, I set the min quantity of an item then tried checking out. It shows the snackbar notice. Setting an incorrect postcode also shows a snackbar. |
0b24513
to
39fa3af
Compare
Remove this when supported in Gutenberg.Remove this when supported in Gutenberg. github.com/WordPress/gutenberg/pull/44059
woocommerce-blocks/assets/js/base/utils/create-notice.ts Lines 88 to 100 in 3530786
🚀 This comment was generated by the automations bot based on a
|
Cart errors need to be watched for and created as notices...Cart errors need to be watched for and created as notices elsewhere.
woocommerce-blocks/assets/js/blocks/cart/inner-blocks/filled-cart-block/frontend.tsx Lines 27 to 30 in 3530786
🚀 This comment was generated by the automations bot based on a
|
@opr I've merged the other notice PR here now - there are 2 tasks blocking this from merging because I don't want to surface too many inline notices as you populate the checkout. The worse ones are email and postcode. |
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.
@mikejolley sorry to be pedantic but could we rename the directory or store key for the new notices store? Currently the directory names map to the store name, prefixed with wc/store
. Based on this folder name I, the data store should be wc/store/store-notices
. So let's either update the key or dir name.
3530786
to
fc95e40
Compare
5f4f70f
to
fc66f01
Compare
The release ZIP for this PR is accessible via:
|
@opr This is rebased now, and I've added a description of all the changes I've introduced. There are quite a few files touched, but the changes are mostly minimal. Mostly typescript. Main changes are new contexts, the merging of notice containers, and the grouping of notices by type. |
* Update API type defs * Move create notice utils * Replace useCheckoutNotices with new contexts * processCheckoutResponseHeaders should check headers are defined * Scroll to error notices only if we're not editing a field * Error handling utils * processErrorResponse when pushing changes * processErrorResponse when processing checkout * remove formatStoreApiErrorMessage * Add todo for cart errors * Remove unused deps * unused imports * Fix linting warnings * Unused dep * Update assets/js/types/type-defs/api-response.ts Co-authored-by: Thomas Roberts <5656702+opr@users.noreply.github.com> * Add todo * Use generic * remove const * Update array types * Phone should be in address blocks Co-authored-by: Thomas Roberts <5656702+opr@users.noreply.github.com>
6aacbfc
to
c47265f
Compare
This PR refactors
StoreNoticeContainer
so it combines snack bars and regular notices (instead of separate components) and introduces newnoticeContexts
for inner blocks.New noticeContexts
Adds several notice contexts for the main inner blocks that can be used as
context
instead of strings. Each notice context has a correspondingStoreNoticesContainer
component to render any notices added.Rather than create a new interface, I extended the existing one which only contained payment contexts:
The full list is now:
Notice grouping
Groups notices of the same type into a single error notice. You can test this on the cart page by calling this a few times:
Notice creation helpers
Existing notices have been modified to use the new containers and
createNotice
functions.Additionally, when a new error is displayed, it will scroll to the parent
StoreNoticeContainer
. The scrolling fixes #2559This mechanism also ensures that if a
StoreNoticesContainer
does not exist, it uses a parent context.Changes to hooks
New filters:
showApplyCouponNotice
andshowRemoveCouponNotice
To simplify notice handling, I've moved the error suppression for coupons to
showApplyCouponNotice
to prevent the notice from being added in the first place. This will break Points and Rewards (in a non-destructive way), so this will need a patch to use the new filter (https://github.com/woocommerce/woocommerce-points-and-rewards/blob/5d3b6812188e969505f612868ce228a387f1ea12/assets/js/filters.js#L24).The fix for points and rewards can be found here: https://github.com/woocommerce/woocommerce-points-and-rewards/pull/491
Other changes
useCheckoutNotices
. It was only used in one place and not shared, so I moved the logic inline.removeAllNotices
when processing checkout so new notices can surface.processErrorResponse
andprocessInvalidParamResponse
from the APICloses #2559
Testing
User-facing testing
Snack bars:
Error notices from API:
cc @opr