-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Block bindings: Add support for image caption #6838
base: trunk
Are you sure you want to change the base?
Changes from 14 commits
f584ad4
7d7d5c3
731d869
c360edf
afbf38c
cbe79ee
083c4e2
01d0e86
632e015
79013a9
cf25d7d
88df640
1c4ac72
ac43482
19465c0
1855e5d
4e77262
825f78e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -246,7 +246,7 @@ private function process_block_bindings() { | |
$supported_block_attributes = array( | ||
'core/paragraph' => array( 'content' ), | ||
'core/heading' => array( 'content' ), | ||
'core/image' => array( 'id', 'url', 'title', 'alt' ), | ||
'core/image' => array( 'id', 'url', 'title', 'alt', 'caption' ), | ||
'core/button' => array( 'url', 'text', 'linkTarget', 'rel' ), | ||
); | ||
|
||
|
@@ -333,6 +333,60 @@ private function replace_html( string $block_content, string $attribute_name, $s | |
switch ( $block_type->attributes[ $attribute_name ]['source'] ) { | ||
case 'html': | ||
case 'rich-text': | ||
if ( 'core/image' === $this->name && 'caption' === $attribute_name ) { | ||
// Create private anonymous class until the HTML API provides `set_inner_html` method. | ||
$bindings_processor = new class( $block_content, WP_HTML_Processor::CONSTRUCTOR_UNLOCK_CODE ) extends WP_HTML_Processor { | ||
SantosGuillamot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
/** | ||
* Replace the inner content of a figcaption element with the passed content. | ||
* | ||
* DO NOT COPY THIS METHOD. | ||
* THE HTML PROCESSOR WILL HAVE A PROPER METHOD. | ||
* USE IT INSTEAD. | ||
* | ||
* @since 6.7.0 | ||
* | ||
* @param string $new_content New content to insert in the figcaption element. | ||
* @return bool Whether the inner content was properly replaced. | ||
*/ | ||
public function set_figcaption_inner_text( $new_content ) { | ||
// Check that the processor is paused on an opener tag. | ||
if ( 'FIGCAPTION' !== $this->get_tag() || $this->is_tag_closer() ) { | ||
return false; | ||
} | ||
|
||
// Once this element closes the depth will be one shallower than it is now. | ||
$depth = $this->get_current_depth(); | ||
while ( $this->next_token() && $this->get_current_depth() >= $depth ) { | ||
// This is inside the FIGCAPTION element. | ||
} | ||
|
||
if ( null !== $this->get_last_error() || $this->paused_at_incomplete_token() ) { | ||
return false; | ||
} | ||
SantosGuillamot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
$this->set_bookmark( 'here' ); | ||
|
||
$opening = $this->bookmarks[ $this->current_element->token->bookmark_name ]; | ||
$closing = $this->bookmarks['_here']; | ||
$start = $opening->start + $opening->length; | ||
|
||
$this->lexical_updates[] = new WP_HTML_Text_Replacement( | ||
$start, | ||
$closing->start - $start, | ||
wp_kses_post( $new_content ) | ||
SantosGuillamot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
); | ||
|
||
return true; | ||
} | ||
}; | ||
|
||
$block_reader = $bindings_processor::create_fragment( $block_content ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. whoah. this is unrelated to your changes @SantosGuillamot, but this line really confuses me, purely based on how PHP's anonymous classes work. @sirreal maybe what I wrote above is fine, and when we call yes, I just tested and this is odd. calling @SantosGuillamot I wonder if we could take advantage of this in line 338 and instead of passing There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not 100% this is what you meant, but I made this change to address the feedback. Let me know if that's not what you were suggesting. |
||
if ( $block_reader->next_tag( 'figcaption' ) ) { | ||
$block_reader->set_figcaption_inner_text( $source_value ); | ||
} | ||
return $block_reader->get_updated_html(); | ||
} | ||
|
||
$block_reader = new WP_HTML_Tag_Processor( $block_content ); | ||
|
||
// TODO: Support for CSS selectors whenever they are ready in the HTML API. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is an interesting side-effect of that choice. I think we should rename that constant to something scarier, like what its value is.
I kind of like that it makes anonymous classes stand out more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sirreal I think this is a really good example of a part of the HTML Processor we're going to have to improve.
not only is the unlock code ugly here (ugly, because it shouts how this is work-around territory), but also by calling the constructor directly we bypass setting up the context node and the HTML node. in effect, this is starting as a full parser instead of a fragment parser.
I wonder how we can better allow subclassing without this hassle and risk.