-
Notifications
You must be signed in to change notification settings - Fork 219
Add CartEventContext
and dispatch events when pressing proceed to checkout button
#7809
Conversation
The release ZIP for this PR is accessible via:
|
TypeScript Errors ReportFiles with errors: 436
assets/js/base/context/providers/cart-checkout/cart-events/test/index.tsx
assets/js/blocks/cart/inner-blocks/proceed-to-checkout-block/test/block.tsx |
Size Change: +934 B (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
dd62544
to
c43994d
Compare
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.
Thank you for working on this Thomas. I think what this PR is missing is a why section? why do we want to trigger an event on what supposed to be a regular link transition between two pages? I think this button serves a different purpose than say Place Order which puts things into motion. I'm overall not opposing this PR, just trying to understand some valid use cases for this.
Do we also need to document this somewhere?
> | ||
{ __( 'Proceed to Checkout', 'woo-gutenberg-products-block' ) } | ||
</Button> | ||
<div> |
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.
Was this extra div always here?
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.
No, good catch I don't know why that's here.
The release ZIP for this PR is accessible via:
|
Thank you for the review, Nadir. @mikejolley and I spoke about this (the why) and for now I plan to leave this PR until the rest of the cart validation/error handling is finished. Once it is we can decide if we this PR. At that point I will be able to answer you better about why it's necessary. |
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. |
Closing because this is no longer needed, we are using snackbars to show quantity changes. This PR was supposed to handle the case where validation errors needed to show in the quantity input fields, but this is no longer required. |
d5119bf
to
97e3a32
Compare
The release ZIP for this PR is accessible via:
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
assets/js/base/context/providers/cart-checkout/cart-events/test/index.tsx
|
9557d04
to
628ad57
Compare
Hey @opr is this back?
Whats the new status? |
@mikejolley it's from woocommerce/woocommerce#42648 - the button should not be disabled, but we should still catch and prevent the default action to proceed to checkout. The only way to do this is using this event/observer. |
@opr I tried your example but after hitting cancel it still redirected. Not sure if I did something wrong? |
@mikejolley Weird, I've updated the example in the PR body to add the registration function to the useEffect's dependencies. |
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.
Works with the updated snippet. Approving 👍🏻
This is a re-do of #7694 but it targets
trunk
instead.Testing
Automated Tests
Internal dev testing
woocommerce-blocks/assets/js/base/components/quantity-selector/index.tsx
Line 77 in aa78099
onProceedToCheckout
event, and show a confirmation box. Pressing cancel will cause the observer to return an error object, this should abort the proceed to checkout aciton.confirm
will happen multiple times).User Facing Testing
WooCommerce Visibility
Performance Impact
Changelog