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

Blocks: Introduce a variation of serialize blocks helper with traversing #5246

Closed

Conversation

gziolo
Copy link
Member

@gziolo gziolo commented Sep 19, 2023

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

Reverts changes from #5187 (changeset, ticket) and implements changes discussed in #5242.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@gziolo gziolo force-pushed the update/traverse-and-serialize-block branch from 1a709d1 to 40d6697 Compare September 19, 2023 10:18
src/wp-includes/blocks.php Outdated Show resolved Hide resolved
…_block(s) callback argument

See WordPress#5242.
Co-authored-by: ockham <ockham@git.wordpress.org>
gziolo and others added 2 commits September 19, 2023 13:33
Co-authored-by: Mukesh Panchal <mukeshpanchal27@users.noreply.github.com>
src/wp-includes/blocks.php Outdated Show resolved Hide resolved
Copy link
Contributor

@ockham ockham left a comment

Choose a reason for hiding this comment

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

One final note; otherwise LGTM!

*
* @param string $original Original block markup.
*/
public function test_traverse_and_serialize_identity_from_parsed( $original ) {
Copy link
Member

Choose a reason for hiding this comment

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

For better placement could you add this test after test_serialize_identity_from_parsed?

Copy link
Member Author

Choose a reason for hiding this comment

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

What's the rationale behind it? Are there any recommendations regarding the order of functions in test files? It's now located next to other tests covering traverse_and_serialize_blocks.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mukeshpanchal27, I landed these changes, but I'm happy to work on a follow-up once you respond to the comment. There are two follow-up commits planned to finalize backporting Block Hooks feature from Gutenberg by @ockham that depend on this work.

@gziolo
Copy link
Member Author

gziolo commented Sep 19, 2023

Committed with https://core.trac.wordpress.org/changeset/56620.

@gziolo gziolo closed this Sep 19, 2023
@gziolo gziolo deleted the update/traverse-and-serialize-block branch September 19, 2023 12:52
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