Skip to content
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

Update WCEU workshop interactivity example with WP 6.5 syntax #122

Conversation

luisherranz
Copy link
Member

@verytwisty helped me update the syntax of the WCEU workshop blocks to the final version of the Interactivity API included in WP 6.5.

This pull request brings that update to the example shared in this repository.

@juanmaguitar I haven't tested this locally, but I believe there's a way to test this using the Playground with a link generated directly in this repository. Is that the case?

@juanmaguitar
Copy link
Collaborator

juanmaguitar commented Jul 2, 2024

@juanmaguitar I haven't tested this locally, but I believe there's a way to test this using the Playground with a link generated directly in this repository. Is that the case?

Well, there's not really a way to test this locally using the blueprint.json on each example.
It can be quickly tested with npx @wp-now/wp-now start from each example folder though (more ways to test the examples are explain here).

The blueprints work for code in the repo. The way it works is:

  • the blueprint for the example is available in the repo
  • the blueprint uses a .zip for the example (for the plugin that registers these block) that is automatically generated on every merge to trunk
  • the links to the playground using the custom playground for each example are available from the README of each example (this one for example) or from the table listing all the examples at the README of the repo

@luisherranz
Copy link
Member Author

There is something strange happening. If the quiz blocks are inside the quiz progress block, the HTML breaks and they don't work. This also happens in trunk. I am going to investigate.

@luisherranz
Copy link
Member Author

It's the wp_kses_data here:

<?php echo wp_kses_data( $content ); ?>

I'm going to delete it and add an exception.


Why do you need so many calls to wp_kses_data by the way? We don't have that requirement in Gutenberg… 🤔

@luisherranz luisherranz marked this pull request as ready for review July 2, 2024 18:42
@luisherranz
Copy link
Member Author

Everything is ready 👍

@juanmaguitar
Copy link
Collaborator

juanmaguitar commented Jul 3, 2024

Why do you need so many calls to wp_kses_data by the way? We don't have that requirement in Gutenberg… 🤔

As recommended in Common APIs Handbook > Security > Escaping Data we're escaping all output. As per this using wp_kses_data seems to be a good way to doing so.

The repo follows WordPress Coding Standards and it uses phpcs to sniff the code and check it follows this standard.

If any output is not escaped, the sniffer throws the following message

All output should be run through an escaping function (see the Security sections in the WordPress Developer Handbooks)

Why do you need so many calls to wp_kses_data by the way? We don't have that requirement in Gutenberg… 🤔

Curious about this. I think gutenberg is using a specific ruleset for Core development which seems to be less restrictive than the generic WordPress one

@ryanwelcher any thoughts?

Note

I have opened #123 to continue the conversation there

@juanmaguitar juanmaguitar merged commit 6efcc35 into WordPress:trunk Jul 3, 2024
3 checks passed
@luisherranz luisherranz deleted the update-wceu-workshop-interactivity-example branch July 3, 2024 07:29
>
<div>
<strong><?php echo wp_kses_data( __( 'Answered' ) ); ?></strong>:
<span data-wp-text="state.answered"></span> / <span data-wp-text="state.totalQuizzes"></span>
<span data-wp-text="state.answered"></span>/<?php echo count( $state['quizzes'] ); ?>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@luisherranz the portion of code <?php echo count( $state['quizzes'] ); is throwing an error
I temporarily fixed the error by adding this and this

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

state.quizzes.length is not dynamically changed in the client, so it should be read from a static PHP variable instead of a client getter.

Apart from that, we need that value in the server so it's properly server-side rendered.

What's the exact error that it throwed in production?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By the way, it doesn't work right now because state.quizzes is an object, so it should be Object.keys(state.quizzes).length.

But, anyway, let's fix it properly 😄

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the error I get with <?php echo count( $state['quizzes'] ); ?> on line 33

Answered: /
Warning: Undefined array key "quizzes" in /var/www/html/wp-content/plugins/interactivity-api-quiz-1835fa/build/blocks/quiz-progress-1835fa/render.php on line 33

Fatal error: Uncaught TypeError: count(): Argument #1 ($value) must be of type Countable|array, null given in /var/www/html/wp-content/plugins/interactivity-api-quiz-1835fa/build/blocks/quiz-progress-1835fa/render.php:33 Stack trace: #0 /var/www/html/wp-includes/blocks.php(519): require() #1 /var/www/html/wp-includes/class-wp-block.php(463): {closure}(Array, '', Object(WP_Block)) #2 /var/www/html/wp-includes/blocks.php(1705): WP_Block->render() #3 /var/www/html/wp-includes/blocks.php(1743): render_block(Array) #4 /var/www/html/wp-includes/class-wp-hook.php(324): do_blocks('<!-- wp:block-d...') #5 /var/www/html/wp-includes/plugin.php(205): WP_Hook->apply_filters('<!-- wp:block-d...', Array) #6 /var/www/html/wp-content/plugins/gutenberg/build/block-library/blocks/post-content.php(50): apply_filters('the_content', '<!-- wp:block-d...') #7 /var/www/html/wp-includes/class-wp-block.php(463): gutenberg_render_block_core_post_content(Array, '<!-- wp:block-d...', Object(WP_Block)) #8 /var/www/html/wp-includes/class-wp-block.php(443): WP_Block->render() #9 /var/www/html/wp-includes/blocks.php(1705): WP_Block->render() #10 /var/www/html/wp-includes/blocks.php(1743): render_block(Array) #11 /var/www/html/wp-includes/block-template.php(260): do_blocks('<!-- wp:templat...') #12 /var/www/html/wp-includes/template-canvas.php(12): get_the_block_template_html() #13 /var/www/html/wp-includes/template-loader.php(106): include('/var/www/html/w...') #14 /var/www/html/wp-blog-header.php(19): require_once('/var/www/html/w...') #15 /var/www/html/index.php(17): require('/var/www/html/w...') #16 {main} thrown in /var/www/html/wp-content/plugins/interactivity-api-quiz-1835fa/build/blocks/quiz-progress-1835fa/render.php on line 33
There has been a critical error on this website.

[Learn more about troubleshooting WordPress.](https://wordpress.org/documentation/article/faq-troubleshooting/)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, wait. I know what's happening. The quiz blocks need to be inner blocks of the quiz progress block. Can you change the blueprint to be like that? I'll make the PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The quiz blocks need to be inner blocks of the quiz progress block.

@luisherranz In that case, do we need to set the namespace for the children as it is done here?

Also, are independent blocks unable to share namespace and state/store?

Copy link
Member Author

@luisherranz luisherranz Jul 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we need to set the namespace for the children as it is done here?

Yes, even though it's not strictly required, it's a good practice to always add the data-wp-interactive directive.

are independent blocks unable to share namespace and state/store?

What do you mean?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean?

I mean, can two blocks being at the same level (no inner blocks) in different parts of the HTML share namespace and state/store

For example, having two blocks registered by different plugins, can they share namespace (and store/actions) without a parent/child relationship?

<div id="block-registered-by-plugin-1" data-wp-interactive="common-project-namespace">
</div>
<div id="block-registered-by-plugin-1" data-wp-interactive="common-project-namespace">
</div>

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yeah, sure 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants