-
Notifications
You must be signed in to change notification settings - Fork 219
Conversation
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
🎉 🎉 This PR does not introduce new TS errors. |
Blocked by #8110 |
This reverts commit 38d66ed.
078da3a
to
ebd0fa2
Compare
Size Change: +1.82 kB (0%) Total Size: 1.1 MB
ℹ️ View Unchanged
|
I've rebased this and fixed the tests. Going to ask another reviewer to take a look. |
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, tested and it's working nicely. I added a comment about adding links/version numbers to the deprecated message, please can we do this before shipping?
return function ( | ||
...args: Parameters< typeof onCheckoutValidationBeforeProcessing > | ||
) { | ||
return function ( ...args: Parameters< typeof onCheckoutValidation > ) { | ||
deprecated( 'onCheckoutBeforeProcessing', { | ||
alternative: 'onCheckoutValidationBeforeProcessing', |
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.
Should this be updated since we're now deprecating onCheckoutValidationBeforeProcessing
too
return function ( ...args: Parameters< typeof onCheckoutValidation > ) { | ||
deprecated( 'onCheckoutValidationBeforeProcessing', { | ||
alternative: 'onCheckoutValidation', | ||
plugin: 'WooCommerce Blocks', |
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.
Should we add links and version numbers to these deprecation messages?
This PR follows on from #8110 , where we refactored the payment status (mostly) and the checkout status. We drew diagrams and flows and went deep about how the checkout flow should look in terms of events and statuses. There was a lot that was confusing and one of those things were the checkout events.
So I've made the following changes here:
onCheckoutValidationBeforeProcessing
. toonCheckoutValidation
onCheckoutAfterProcessingWithSuccess
toonCheckoutSuccess
onCheckoutAfterProcessingWithError
toonCheckoutFail
onPaymentProcessing
tooPaymentSetup
Hopefully this is clearer to everyone going forward.
Here is what the happy path will look like after this PR and #8110
Testing
User Facing Testing
WooCommerce Visibility
Performance Impact
Dev Note
Some of the checkout event emitters have been renamed to better reflect their purpose. Deprecation is in place, so if you’re using the old emitter names your code will continue to work, but you may see a notice in the browser error console. The changes include are as follows.
onCheckoutValidationBeforeProcessing has been renamed to onCheckoutValidation . There is only one validation event during checkout so this makes it clearer where validation listeners should fire.
onCheckoutAfterProcessingWithSuccess has been renamed simply to onCheckoutSuccess since the processing part is not relevant and has no real meaning in this context. The same for onCheckoutAfterProcessingWithError which has dropped the prefix to become onCheckoutFail .
Finally, onPaymentProcessing has been renamed to onPaymentSetup . This is because it was unclear that no actual processing would take place during this event. The event emitter is only there to prepare the payment gateway to take payment when the checkout processes. onPaymentSetup reflects this more clearly.
Changelog