gutenberg icon indicating copy to clipboard operation
gutenberg copied to clipboard

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

Open adamziel opened this issue 2 years ago • 1 comments

A part of https://github.com/WordPress/gutenberg/issues/44410. Supersedes https://github.com/WordPress/gutenberg/pull/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 avatar Oct 18 '22 22:10 adamziel

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

codesandbox[bot] avatar Oct 18 '22 22:10 codesandbox[bot]

@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 avatar Feb 07 '23 13:02 adamziel