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

Issue-64: extract pre_render_block logic #78

Merged

Conversation

anubisthejackle
Copy link
Contributor

@anubisthejackle anubisthejackle commented Apr 29, 2024

Summary

This pull request addresses the issue described in #64. It aims to refactor the pre_render_block filter logic used in both is-false and is-true blocks into reusable functions to adhere to the DRY (Don't Repeat Yourself) principle.

Fixes #64

Changes

  • Extracted the common logic from the pre_render_block filter into separate, reusable functions.
  • Updated the is-false and is-true block implementations to use the new reusable functions.

Testing

  • Added unit tests for the new functions.
  • Updated existing tests to reflect the refactored code.
  • Manually tested the changes in a local development environment to ensure the is-false and is-true blocks behave as expected.

Documentation

  • Updated the README.md if necessary.
  • Added inline documentation to the new functions for clarity.

Notes

  • Any additional information about the changes.

@anubisthejackle anubisthejackle marked this pull request as ready for review April 29, 2024 19:43
Copy link
Member

@kevinfodness kevinfodness left a comment

Choose a reason for hiding this comment

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

Left some questions and suggestions, no blockers 🍣

@@ -27,36 +32,10 @@ function wp_conditional_blocks_is_false_block_init(): void {
* Short-circuit the display of blocks inside if the outer condition isn't false.
*
* @param string|null $pre_render The pre-rendered content. Default null.
* @param WP_Block $parsed_block The block being rendered.
* @param mixed[] $parsed_block The block being rendered.
Copy link
Member

Choose a reason for hiding this comment

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

Can $parsed_block contain other items in the array besides WP_Block objects? What are they?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Parsed block is an array containing (at times) the blockName, attrs, innerBlocks, innerHTML, and innerContent of the block we are rendering. Its never a WP_Block object, as far as I can tell.

Comment on lines +21 to +25
if ( ! isset( $parsed_block['blockName'] ) ) {
return false;
}

return $block_name === $parsed_block['blockName'];
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ( ! isset( $parsed_block['blockName'] ) ) {
return false;
}
return $block_name === $parsed_block['blockName'];
return isset( $parsed_block['blockName'] ) && $block_name === $parsed_block['blockName'];

Comment on lines +37 to +41
if ( ! ( $parent_block instanceof WP_Block ) ) {
return false;
}

return is_active_block( $block_name, $parent_block->parsed_block );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ( ! ( $parent_block instanceof WP_Block ) ) {
return false;
}
return is_active_block( $block_name, $parent_block->parsed_block );
return $parent_block instanceof WP_Block && is_active_block( $block_name, $parent_block->parsed_block );

$context['postId'] = $parent_block->context['postId'];
}

if ( wp_conditional_blocks_condition_block_result( $parent_block->parsed_block, $context ) !== $bool ) { // @phpstan-ignore-line
Copy link
Member

Choose a reason for hiding this comment

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

Why is PHPStan unhappy here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its not able to tell that $parent_block is an object because the check happens in a function.

@anubisthejackle anubisthejackle merged commit 8080928 into develop May 16, 2024
5 of 7 checks passed
@anubisthejackle anubisthejackle deleted the feature/issue-64/extract-pre-render-block-logic branch May 16, 2024 13:36
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.

extract pre_render_block logic
2 participants