-
Notifications
You must be signed in to change notification settings - Fork 219
WIP - Checkout i2 Feature Branch Tracking #4268
Conversation
Size Change: +102 kB (+11%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
60e5232
to
c0251f7
Compare
e4414a0
to
869172c
Compare
754e2a5
to
08d27d8
Compare
Remove custom shipping_phone handling (requires WC 5.6+)Remove custom shipping_phone handling (requires WC 5.6+) https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/e62c42b6858e26e9e7685c4666ca795e311e79f8/src/StoreApi/Routes/CartUpdateCustomer.php#L123-L132🚀 This comment was generated by the automations bot based on a
|
Disable custom locking support if native support is detec...Disable custom locking support if native support is detected. https://github.com/woocommerce/woocommerce-gutenberg-products-block/blob/a3c4fb01f88f308b263f5e99df0cb76c4abc84c9/assets/js/blocks/cart-checkout/checkout-i2/hacks.ts#L85-L97🚀 This comment was generated by the automations bot based on a
|
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.
Thanks for handling the feedback Mike (especially around the concerns for dangerouslySetInnerHTML usage)! I still have a question around the potential (that I see) for the registered blocks array in a checkout area to be mutated. I'm okay with it being handled in a followup if you agree something should be done.
Also, there's an open question I asked in my previous review that I don't see an answer for:
I am wondering, with the pending L2->L0 policy change (likely merged this week), should we gate checkout and cart blocks availability to a hard WordPress 5.8 requirement?
The above could be done in a followup too.
With that in mind I'm pre-approving on the assumption my concerns will either be addressed in followups or in this PR before merge.
I think we'll need to discuss this and work in a follow-up. My concern is that this check would be shifting as new versions are released, and it might make more sense to do detection higher up rather than on a per-block basis. For example, having a version check in the isExperimentalBuild function so that experimental functionality requires L0. |
Merging 🥳 |
Yup I'm fine with a followup!
Right that makes sense while something is still behind the experimental flag. As soon as it is surfaced in the plugin though I think we'll want to gate C&C blocks to the WP version supported at the time of release. Mostly because I'm expecting we will probably be surfacing to new WooCommerce users in Woo Core while Woo Core still has L2 support outside the range of what the blocks will require. But that's also something we could also plan for when we come to it I suppose. My primary concern is that we whenever this is surfaced to feature plugin users we have some way of indicating what is the minimum version this will require to help inform the approach we take when we plan for surfacing in WooCommerce core. |
We'd like to merge the new Checkout i2 block into trunk flagged "experimental" to avoid this PR getting much larger now that many of the features are implemented. The block itself is not yet complete.
You can scan over the Checkout i2 block files if you wish, but as it's incomplete, focussing on the non-Checkout i2 block changes would be best when reviewing, since those parts can impact other blocks.
Closes #4444
This is a summary of changes outside of the Checkout i2 block that should be reviewed:
shipping phone
in addition tobilling phone
. Both can be sent via the API, and both will be stored to meta data. The different values will not be visible in admin until core support is added.blocks-registry
- This is consumed by Checkout i2 to register (experiential) blocks for insertion into checkout areas. It's the extensibility interface. This should not impact other blocks, and this has test coverage.ExternalLinkCard
- Editor component used by i2 to link to WordPress admin pages.withScrollToTop
HOC - Converted to a functional component to avoid Typescript errors.CustomerDataContext
- Moved some methods to this context so they are available outside of component state.useCheckoutAddress
- Support for shipping/billing phoneShippingRatesControl
- Avoid slot fill when previewing, because the slot fill is only available on the frontend.Testing