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.

Sign up for GitHub

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

Jump to bottom

strip whitespaces in render_block_core_cover #40859

Merged
merged 10 commits into from
Jun 27, 2022
Merged

strip whitespaces in render_block_core_cover #40859

Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
09739d8
strip whitespaces in render_block_core_cover
navigatrum May 5, 2022
849197a
amend regex
navigatrum May 18, 2022
9be451c
Cover-block: edit regex to insert the featured img
navigatrum May 26, 2022
ee8fe32
edit cover block regex for featured image #4
navigatrum Jun 15, 2022
2981dd7
Add quotes around the regexp
adamziel Jun 23, 2022
e374216
close bracket ad reformat comment
navigatrum Jun 24, 2022
7d65028
retry to fix
navigatrum Jun 25, 2022
98424c7
Update packages/block-library/src/cover/index.php
adamziel Jun 27, 2022
7cd71a2
Update packages/block-library/src/cover/index.php
adamziel Jun 27, 2022
4a4aa98
Merge branch 'trunk' into edit-cover-block-featured-image
adamziel Jun 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions packages/block-library/src/cover/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,13 @@ class="wp-block-cover__image-background"
esc_attr( $object_position )
);

$content = str_replace(
'</span><div',
'</span>' . $image . '<div',
$content
);
/* Inserts the featured image between the (1st) cover 'background' `span` and 'inner_container' `div`,
Copy link
Contributor

@azaozz azaozz May 26, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Why does the featured image have to be inserted there? Who decided that and for what purpose? Could you please comment to make sure we're on the same page (I'm probably missing some considerations there).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably you didn't read the first line of comment to this PR.
It says:

For the block layout see: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/cover/edit/index.js#L327-L367

Fell free to take a look at the history of that file: the who and their purpose cannot be really hidden. My 2¢? A henchman of his. 😄

Copy link
Contributor

@draganescu draganescu May 27, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@azaozz I decided that for not having a better way - as Gutenberg is missing a token system - to make the cover block get the featured image at render time.

If you look at the history of PRs in which I tried to solve the featured image in cover block problem you'll see there are a few constraints that arise from our lack of a token system which results in this kind of render string operations on the block markup.

It is not the only block where these operations occur and I think it also happens (e.g. gutenberg_restore_group_inner_container, render_block_core_image, render_block_core_post_featured_image, render_block_core_template_part, render_block_core_site_logo).

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, my bad, should have explained this better. Was thinking that the way this works may have been simpler if the featured image was appended to the end of the block's HTML. The difference in the featured image's node position would have probably been pretty easy to account for with a little of CSS. Even maybe that could have been preferred in some cases as would have made it possible to determine when an image was inserted there.

and removes eventual withespace characters between the two (typically introduced at template level) */
$inner_container_start = /<div\b[^>]+wp-block-cover__inner-container[\s|"][^>]*>/U;
adamziel marked this conversation as resolved.
Show resolved Hide resolved
if ( 1 === preg_match( $inner_container_start, $content, $matches, PREG_OFFSET_CAPTURE ) {
list( $fullmatch, $offset ) = $matches[0];
$content = substr( $content, 0, $offset ) . $image_tag . substr( $content, $offset );
}

adamziel marked this conversation as resolved.
Show resolved Hide resolved
}

Expand Down