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

Tag Processor: Remove the shorthand next_tag( $tag_name ) syntax #45082

Closed
wants to merge 7 commits into from

Conversation

adamziel
Copy link
Contributor

A part of #44410. Supersedes #44595 due to a branch naming collision

What?

In @dmsnell's words:

My worry is that we say you can search for a given tag name, but then people want a short hand for class name, so we add it, but then if you don't specify a tag you have ->next( null, 'wp-block-group' ) everywhere. Or someone thinks, let's put class name first, and then we have the other problem.

I'd even be for exposing this shorthand only after we see use in practice to see if people even want it, and if so, which convention is more dominant.

Alternatively it's always easy to later add new specialized methods. Suppose we only expose ->next() right now, taking a query object. we can add ->next_tag( 'img' ) and ->next_with_class( 'wp-block-group' ). performance wise this is a "free" option

It resonates with me, so this PR removes the shorthand syntax and renames the long tag_name and class_name keys to short tag and class ones.

Before:

$p->next_tag( 'p' );

After:

$p->next( array( 'tag' => 'p' ) );

Testing Instructions

Confirm the CI passed and that no next_tag, tag_name, and class_name mentions were missed in this refactoring.

cc @dmsnell

@adamziel adamziel added [Type] Enhancement A suggestion for improvement. Developer Experience Ideas about improving block and theme developer experience labels Oct 18, 2022
@adamziel adamziel self-assigned this Oct 18, 2022
@codesandbox
Copy link

codesandbox bot commented Oct 18, 2022

CodeSandbox logoCodeSandbox logo  Open in CodeSandbox Web Editor | VS Code | VS Code Insiders

@adamziel adamziel mentioned this pull request Dec 1, 2022
26 tasks
@gziolo gziolo added the [Feature] Block API API that allows to express the block paradigm. label Jan 31, 2023
@adamziel
Copy link
Contributor Author

adamziel commented Feb 7, 2023

@dmsnell since WP_HTML_Tag_Processor is now merged to core and feature freeze is due today, I don't think we can progress with this PR anymore. I'm closing for now – feel free to reopen if you disagree.

@adamziel adamziel closed this Feb 7, 2023
@gziolo gziolo deleted the cleaup-tag-processor-api branch February 7, 2023 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience [Feature] Block API API that allows to express the block paradigm. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants