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

WP_Theme::is_block_theme(): Add more methods of identifying a block theme. #2240

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from

Conversation

costdev
Copy link
Contributor

@costdev costdev commented Jan 28, 2022

Based on the paths outlined by @anton-vlasenko. Includes unit tests. Ready for review.

Trac ticket: https://core.trac.wordpress.org/ticket/54910

@costdev costdev force-pushed the is_block_theme_revised branch 2 times, most recently from 2795b2a to 63d43cd Compare January 28, 2022 19:34
@costdev costdev marked this pull request as ready for review January 28, 2022 20:14
continue;
}

$blocks = parse_blocks( trim( $content ) );
Copy link
Contributor Author

@costdev costdev Jan 28, 2022

Choose a reason for hiding this comment

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

Without trim(), parse_blocks() will interpret an empty last line as a block with blockName => NULL, which could lead to a false negative.

Copy link
Member

Choose a reason for hiding this comment

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

questions (non-blocking): Is there a reason we're parsing the blocks? The str_contains should be enough to check the existence of the blocks.

The has_blocks uses a similar check as well - https://developer.wordpress.org/reference/functions/has_blocks/.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hybrid themes may use mixed HTML (block and non-block markup). The thinking here is that parsing the blocks is a way to determine whether this is a "pure" block template or not.

The str_contains() is a quick way for us to determine if there are any blocks. If there are no blocks, we can return false and save the resources required to parse the contents more thoroughly.

Copy link

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

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

LGTM! Nice job on the unit tests.

@costdev
Copy link
Contributor Author

costdev commented Feb 1, 2022

Removed the fix for get_query_template() as this turned out to be the fix for another ticket and is more appropriate to include there instead.

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.

3 participants