-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hydrate Cart Order Summary Coupon Form inner block using BHE #54
Hydrate Cart Order Summary Coupon Form inner block using BHE #54
Conversation
Size Change: +1.03 kB (0%) Total Size: 873 kB
ℹ️ View Unchanged
|
Hey @c4rl0sbr4v0, to prevent the diff --git a/assets/js/atomic/utils/render-parent-block.tsx b/assets/js/atomic/utils/render-parent-block.tsx
index bb6b2ecd..19d3eff4 100644
--- a/assets/js/atomic/utils/render-parent-block.tsx
+++ b/assets/js/atomic/utils/render-parent-block.tsx
@@ -141,7 +141,20 @@ const renderInnerBlocks = ( {
if ( ! children || children.length === 0 ) {
return null;
}
- return Array.from( children ).map( ( node: Node, index: number ) => {
+ Array.from( children ).map( ( node: Node, index: number ) => {
+ /**
+ * Do not process the node if its a `<wp-block>` element.
+ */
+ if (
+ node instanceof HTMLElement &&
+ node instanceof window.customElements.get( 'wp-block' )
+ ) {
+ const reactElement = parse( node.outerHTML );
+
+ if ( isValidElement( reactElement ) )
+ return cloneElement( reactElement );
+ }
+
/**
* This will grab the blockName from the data- attributes stored in block markup. Without a blockName, we cannot
* convert the HTMLElement to a React component. That would return the JSX-representation of the |
src/BlockTypesController.php
Outdated
$block_type = $instance->block_type; | ||
if ( ! $block_type ) { | ||
// We might use a better way to flag it here. | ||
wc_add_notice( 'You need to make a server registration of ' . $block['blockName'], 'error' ); |
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.
Maybe we can use _doing_it_wrong()
here? Although that's supposed to flag a function that's invoked in the wrong way; not totally sure that applies here 🤔
Although maybe it's arguable that the render_block
hook should be called with 3 args? 🤔
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.
As @tarhi-saad just told me, they server-registered all cart inner blocks in this PR in order to fix a translation issue.
I will try to update the repo. So that way it would be fixed. Then we can discuss about adding a flag or not 😆
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.
Ah! Of course, he told me that too 😆 I'd just forgotten about it 😅
3e0f77c
to
1c18a86
Compare
1c18a86
to
58adfc6
Compare
…ror at pre-commit
4383989
to
29df37c
Compare
We might also want to consider if we should change a block's Lines 31 to 33 in cbaf58c
We might want to change that to something more meaningful (that's closer to the frontend code). |
In this case, our registration is adding the I used our frontend and seems to be working. Except when you reload the post editor, I'm taking a look at it 😄 |
The error is due to the fact that the block is using We have it documented here: |
Script Dependencies ReportThe
This comment was automatically generated by the |
Although we are not preventing the hydration from the parent block. I consider this PR ready for review.
|
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.
I'm actually a bit surprised that this works 😄
There is a context (useValidationContext
) that won't have a provider until that block is also moved to BHE, but other than that and the fact that useBlockEnvironment
should not be needed in the future, it looks fine to me.
After seeing this in action, I've started thinking that Woo would probably want to keep the Save component of these blocks as mere <div>
tags to avoid the problems of static block deprecations. If they use the View/Frontend component to do static rendering, they will have mismatches when they update the markup and people would have to go to the dashboard, open the Cart/Checkout page, and save the HTML again. If they use just a <div>
tag (client-side rendering only), they won't have problems. Client-side rendering would cause layout shifts while loading the Cart and Checkout, though.
As mentioned on the tracking issue #20
This PR tries to hydrate the Cart Order Summary Coupon Form inner block by using our Block Hydration experiments.
Fixes #66
registerBlockType()
.