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

Fix image captions hiding rounded borders in galleries #45563

Conversation

artemiomorales
Copy link
Contributor

What?

This pull request addresses issue #43586.
It fixes an issue wherein the caption background of image blocks within Galleries that have a border-radius obscure the bottom corners of the image.

Note: This pull request is not yet meant to be a final implementation; I'm hoping to get feedback on how best to proceed.

Why?

As a user, I expect that if I have a border radius defined, that the image captions will be styled so as to retain my desired border effects.

How?

  1. This request adds a filter to the Gallery block that checks to see if a border-radius is defined on the images via the theme.json
  2. If so, it stores the value of the border-radius as an attribute on the image blocks.
  3. The image blocks, if the border-radius attribute is set, apply it, as well as the overflow: hidden style, to the <figure> elements wrapping their <img> elements via render_callback.

Note: This is not yet working in the Gutenberg editor and only works for saved markup at the moment.

Testing Instructions

  1. Add a border radius to the Image block via theme.json:
{
	"styles": {
		"blocks": {
			"core/image": {
				"border": {
					"radius": "15px"
				}
			}
		}
	}
}
  1. Test by adding a Gallery block with at least one image that has a caption.
  2. Preview the post to see the caption background appearing as expected.

Screenshot

Screen Shot 2022-11-04 at 4 04 42 PM

Gallery shows the image captions displaying as expected.

Questions

I was initially trying to find a more elegant solution, hoping to generate styles for the <figure> at the moment the theme.json was parsed. However, not making any headway on that approach, I decided to try this so we could at least have something to look at while discussing next steps.

Am I in the right ballpark with this solution, or should we be trying a different approach?

Regarding the idea of toggling whether a caption is overlaid, raised by @aaronrobertshaw, I realized that if one selects individual images under the Gallery, one can toggle the style between default and rounded. Setting the style to rounded moves the caption and fixes the issue:

Screen Shot 2022-11-04 at 4 16 06 PM

Should I add a toggle to the Gallery block that switches between this default and rounded setting in its inner image blocks?

Please let me know your thoughts! I'm also happy to try a different approach entirely.

…of a gallery

When a gallery image contains a border radius and captions, the caption
container covers up the rounded corners of an image.

This commit checks to see if the border radius is set on core/image,
and if so, applies styles to the <figure /> element that will allow
captions to render properly.
@codesandbox
Copy link

codesandbox bot commented Nov 4, 2022

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

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 4, 2022
@github-actions
Copy link

github-actions bot commented Nov 4, 2022

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @artemiomorales! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

}
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

seems like it would make sense to check first if we're working with a gallery block before grabbing the theme json data as that might be the costlier step here.

function block_core_gallery_caption_styles( $block ) {
	if ( 'core/gallery' !== $block['blockName'] ) {
		return $block;
	}

	$styles = …

	if ( ! isset( $styles['styles']['blocks']['core/image']['border']['radius'] ) {
		return $block;
	}

	$border_radius = $styles['styles']['blocks']['core/image']['border']['radius'];
	foreach ( $block['innerBlocks'] as $key => $inner_block ) {
		if ( 'core/image' === $inner_block['block_name'] ) {
			$block['innerBlocks'][ $key ]['attrs']['border-radius'] = $border_radius;
		}
	}

	return $block;
}

Note here too a few styling issues. Some linter will probably also pick these up:

  • PHP code in WordPress follows snake_casing convention. it gets a bit confusing when working at the interface with the JavaScript code here, which follows camelCasing.
  • WordPress prefers spaces everywhere, inside parentheses, etc… You don't need or want any when referencing an array key by literal, but do when accessing by variable.
  • Generally speaking, we prefer early-abort flow to avoid nesting and two be able to mentally stop interpreting a function's flow as soon as we can.

@@ -24,6 +24,11 @@ function render_block_core_image( $attributes, $content ) {
$content = str_replace( '<img', '<img ' . $data_id_attribute . ' ', $content );
}
}

if ( isset( $attributes['border-radius'] ) ) {
$content = str_replace( '<figure', "<figure style='border-radius:" . $attributes['border-radius'] . ";overflow:hidden;'", $content);
Copy link
Member

Choose a reason for hiding this comment

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

despite the lines above using str_replace this might be a good place to use the new tag processor. might be worth modifying the whole function so that it's not split between two approaches.

of note: if the figure already had a style attribute in it then this change would wipe out that style attribute, making it a duplicate attribute that browsers ignore.

function render_block_core_image( $attributes, $content ) {
	if ( isset( $attributes[ 'data-id'] ) ) {
		$tags = new WP_HTML_Tag_Processor( $content );
		/*
		 * The data-id="$id" attribute on the img element provides
		 * backwards-compatibility for the gallery block, which now
		 * wraps image blocks within its inner blocks.
		 *
		 * @see the `render_block_data` hook in the core/gallery block
		 */
		if ( ! $tags->next_tag( 'img' ) ) {
			break;
		}

		$tags->set_attribute( 'data-id', $attributes['data-id'] );
		$content = (string) $tags;
	}

	if ( isset( $attributes['border-radius'] ) ) {
		$tags = new WP_HTML_Tag_Processor( $content );
		if ( ! $tags->next_tag( 'figure' ) ) {
			break;
		}

		$style  = $tags->get_attribute( 'style' );
		$border = "border-radius: {$attributes['border-radius']}; overflow: hidden;";
		$tags->set_attribute( 'style', "{$style}; {$border}" );
		$content = (string) $tags;
	}

	return $content;
}

@Mamaduka Mamaduka added [Block] Gallery Affects the Gallery Block - used to display groups of images [Type] Bug An existing feature does not function as intended [Block] Image Affects the Image Block and removed [Block] Gallery Affects the Gallery Block - used to display groups of images labels Nov 7, 2022
@artemiomorales
Copy link
Contributor Author

As I've looked further into this issue, it appears the best way to move forward would be to get additional design exploration and development to see how we might address gallery caption styles at the global level. Please see this issue for more info; I'll close this pull request for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Image Affects the Image Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants