-
Notifications
You must be signed in to change notification settings - Fork 218
Serialize the Interactivity API's store from PHP and hydrate it on the client #8447
Conversation
check if priority 9 is enough.check if priority 9 is enough.
woocommerce-blocks/src/Interactivity/woo-directives.php Lines 157 to 159 in 9af6478
🚀 This comment was generated by the automations bot based on a
|
change the store tag ID for a better one.change the store tag ID for a better one.
woocommerce-blocks/assets/js/interactivity/store.js Lines 20 to 31 in 9af6478
🚀 This comment was generated by the automations bot based on a
|
Error handling.Error handling.
woocommerce-blocks/src/Interactivity/directives/tags/woo-context.php Lines 16 to 19 in 9af6478
🚀 This comment was generated by the automations bot based on a
|
Retain surrounding whitespace from $style_value, if any.Retain surrounding whitespace from $style_value, if any.
woocommerce-blocks/src/Interactivity/directives/utils.php Lines 33 to 45 in 9af6478
🚀 This comment was generated by the automations bot based on a
|
Add some directive/components registration mechanism.Add some directive/components registration mechanism.
woocommerce-blocks/src/Interactivity/woo-directives.php Lines 110 to 121 in 9af6478
🚀 This comment was generated by the automations bot based on a
|
check if `wp_footer` is the most appropriate hook.check if `wp_footer` is the most appropriate hook.
woocommerce-blocks/src/Interactivity/woo-directives.php Lines 158 to 159 in 9af6478
🚀 This comment was generated by the automations bot based on a
|
Implement evaluation of complex logical expressions.Implement evaluation of complex logical expressions.
woocommerce-blocks/src/Interactivity/directives/utils.php Lines 9 to 20 in 9af6478
🚀 This comment was generated by the automations bot based on a
|
find a better ID for the script tag.find a better ID for the script tag.
woocommerce-blocks/src/Interactivity/directives/class-woo-directive-store.php Lines 27 to 32 in 9af6478
🚀 This comment was generated by the automations bot based on a
|
Do we want to unset styles if they're null? $tags->remove...Do we want to unset styles if they're null? $tags->remove_class( $style_name ); https://github.com/woocommerce/woocommerce-blocks/blob/9af64783ef376034d51ec0c4700e2239bd3257dd/src/Interactivity/directives/attributes/woo-style.php#L25-L30🚀 This comment was generated by the automations bot based on a
|
Error handling.Error handling.
woocommerce-blocks/src/Interactivity/directives/attributes/woo-context.php Lines 16 to 19 in 9af6478
🚀 This comment was generated by the automations bot based on a
|
The release ZIP for this PR is accessible via:
Script Dependencies ReportThe
This comment was automatically generated by the TypeScript Errors Report
assets/js/interactivity/store.js
|
Size Change: +207 B (0%) Total Size: 1.11 MB
ℹ️ View Unchanged
|
Move into `WP_HTML_Tag_Processor` (or `WP_HTML_Processor`).Move into `WP_HTML_Tag_Processor` (or `WP_HTML_Processor`). github.com/WordPress/gutenberg/pull/47573.
woocommerce-blocks/src/Interactivity/directives/woo-process-directives.php Lines 64 to 76 in 97f39c1
🚀 This comment was generated by the automations bot based on a
|
check if `wp_footer` is the most appropriate hook.check if `wp_footer` is the most appropriate hook.
woocommerce-blocks/src/Interactivity/woo-directives.php Lines 125 to 126 in 97f39c1
🚀 This comment was generated by the automations bot based on a
|
Do we want to unset styles if they're null? $tags->remove...Do we want to unset styles if they're null? $tags->remove_class( $style_name ); https://github.com/woocommerce/woocommerce-blocks/blob/97f39c1c138c6a57944ea57f58f7467cf392d424/src/Interactivity/directives/attributes/woo-style.php#L25-L30🚀 This comment was generated by the automations bot based on a
|
Move into `WP_HTML_Tag_Processor` (or `WP_HTML_Processor`).Move into `WP_HTML_Tag_Processor` (or `WP_HTML_Processor`). github.com/WordPress/gutenberg/pull/47573.
woocommerce-blocks/src/Interactivity/directives/woo-process-directives.php Lines 64 to 76 in 0cd9a23
🚀 This comment was generated by the automations bot based on a
|
check if `wp_footer` is the most appropriate hook.check if `wp_footer` is the most appropriate hook.
woocommerce-blocks/src/Interactivity/woo-directives.php Lines 125 to 126 in 0cd9a23
🚀 This comment was generated by the automations bot based on a
|
Do we want to unset styles if they're null? $tags->remove...Do we want to unset styles if they're null? $tags->remove_class( $style_name ); https://github.com/woocommerce/woocommerce-blocks/blob/0cd9a2337f8277ed3c0d9722fb97f7c3f995349d/src/Interactivity/directives/attributes/woo-style.php#L25-L30🚀 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.
The JS part looks good to me.
For the PHP part, maybe @ockham could take a look?
Thanks for taking a look, @luisherranz. BTW, for anyone following this PR: sorry for the noise with the TODO messages; I'll try not to push any of them the next time. 🙇 |
@ockham, could you take a look at the PHP files, just to be sure I ported everything correctly from our repo? Thanks! 🙂 |
|
||
require_once __DIR__ . '/class-woo-directive-store.php'; | ||
|
||
function store( $data ) { |
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.
The function names in this file might later collide with the ones we're using in the Block Interactivity API. Have you considered prefixing them?
} | ||
|
||
// See e.g. https://github.com/WordPress/gutenberg/pull/47573. | ||
function is_html_void_element( $tag_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.
Another function name to potentially prefix 😊
src/Interactivity/woo-directives.php
Outdated
@@ -0,0 +1,123 @@ | |||
<?php | |||
include_once __DIR__ . '/../../../gutenberg/lib/experimental/html/wp-html.php'; |
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.
Note that the files in lib/experimental/html
were recently moved to a different location. You might want to carry over the changes from WordPress/block-interactivity-experiments#162 😅
src/Interactivity/woo-directives.php
Outdated
if ( isset( $instance->parsed_block['isolated'] ) ) { | ||
$w = new WP_HTML_Tag_Processor( $block_content ); | ||
$w->next_tag(); | ||
$w->set_attribute( 'data-woo-ignore', '' ); |
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 think the preferred way of doing this is
$w->set_attribute( 'data-woo-ignore', '' ); | |
$w->set_attribute( 'data-woo-ignore', true ); |
The HTML Tag Processor is smart enough to translate that to
<some-tag data-woo-ignore>
(and not <some-tag data-woo-ignore="true">
, if that was your concern 😄 ).
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, nice! In that case, I think we'd have to change that in our repo as well (I just copied the code from there).
src/Interactivity/woo-directives.php
Outdated
// Add the `data-woo-island` attribute if it's interactive. | ||
$w = new WP_HTML_Tag_Processor( $block_content ); | ||
$w->next_tag(); | ||
$w->set_attribute( 'data-woo-island', '' ); |
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 left a few notes! (Looking good overall though 😊) |
Thanks for your reviews, @luisherranz and @ockham. I will merge this PR even though we don't have explicit approval, as I know @gigitux also reviewed the code. 😁 |
What
Ports latest changes in https://github.com/WordPress/block-hydration-experiments to add store serialization.
Changes included in this PR
wpx
tostore
class
directive when noclass
attribute is presentTracking issue: woocommerce/woocommerce#42486
Why
To keep the Interactivity API in WooCommerce blocks up to date.
How
For the JS part, just copy-pasting the code and updating the values in the
constants.js
file.For the PHP part (after copy-pasting):
wp_
prefixes in global functions withwoo_
(excluding WP's ones, likewp_register_script
)WP_
classes toWoo_
ones (excluding WP's ones, likeWP_HTML_Tag_Processor
)class-wp-
toclass-woo-
wp-
prefixes in directives and replacing them withdata-woo-
(exceptwp-html
, in a require declaration)For all files, removing all TODO comments using a regexp (
([^\n]+)?// TODO.*\n
).