Bug fix: Add duotone class to lightbox experiment

[artemiomorales]

What?

Addresses #50973

Fixes a bug wherein duotone filters were not being applied to images with the lightbox behavior enabled

Why?

Basic functionality should work for image when the lightbox is activated

How?

Rather than putting the duotone class on the parent tag of the block in question, it loops through the elements and adds the class to the <figure> elements.

Note: This uses @SantosGuillamot's suggested code from the following comment on the related issue.

Testing Instructions

1. Enable Gutenberg > Experiments > Core Blocks in the admin
2. Add an image to a post
3. Enable the lightbox by enabling Advanced > Behaviors > Lightbox in the block settings
4. Add a duotone via Styles > Filters > Duotone in the block settings
5. Publish and view the post to see the duotone has been applied to the image
6. Click on the image to open the lightbox and ensure the duotone has been applied there as well

Testing Instructions for Keyboard

N/A

[artemiomorales]

I'm not sure if this change would break duotone in any way.
It looks good to me, but would only add duotone when a <figure> element with the .wp-block-image class is present in the block.

[SantosGuillamot]

I'm not sure if this change would break duotone in any way.

It'd be great to ask someone involved in the duotone to check if it could cause issues somehow. Maybe @ajlende could help here (sorry for the ping if that's not the case)?

[gziolo]

I'm not sure if this change would break duotone in any way.

It'd be great to ask someone involved in the duotone to check if it could cause issues somehow. Maybe @ajlende could help here (sorry for the ping if that's not the case)?

It's definitely Alex who can give the best answer. I personally don't think you can narrow down the lookup to the Image block because block supports for duotone is not not only used with several other core blocks, but it also can be enabled by third-party blocks.

Maybe all you need is to change this selector instead:

gutenberg/packages/block-library/src/image/block.json

Lines 111 to 113 in a3a63c1

	"filter": {
	"duotone": ".wp-block-image img, .wp-block-image .components-placeholder"
	}

[artemiomorales]

I rebased on #51089, which removes the extra <div> around the <figure> element, and it fixes the duotoned image in the content. However, the image in the lightbox is still showing up without the styles.

Maybe all you need is to change this selector instead:

gutenberg/packages/block-library/src/image/block.json

Lines 111 to 113 in a3a63c1

"filter": {
"duotone": ".wp-block-image img, .wp-block-image .components-placeholder"
}

I've tried targeting the overlay markup using this approach and it hasn't worked. Will continue digging around.

[ajlende]

It seems the global styles filter is overriding filters applied via the block attributes which shouldn't happen. You can kind of see in the screenshot the different filter applied in the background.

[ajlende]

Maybe all you need is to change this selector instead
…

I've tried targeting the overlay markup using this approach and it hasn't worked.

The duotone hook adds the generated filter id as a class to the first element in the block content.

gutenberg/lib/class-wp-duotone-gutenberg.php

Lines 882 to 886 in 82f6d03

	// Like the layout hook, this assumes the hook only applies to blocks with a single wrapper.
	$tags = new WP_HTML_Tag_Processor( $block_content );
	if ( $tags->next_tag() ) {
	$tags->add_class( $filter_id );
	}

Then for the generated duotone CSS, it combines that filter id class with the duotone selector in block.json to apply the filter.

gutenberg/lib/class-wp-duotone-gutenberg.php

Lines 673 to 684 in 82f6d03

	// Build the CSS selectors to which the filter will be applied.
	$selectors = explode( ',', $duotone_selector );
	$selectors_scoped = array();
	foreach ( $selectors as $selector_part ) {
	// Assuming the selector part is a subclass selector (not a tag name)
	// so we can prepend the filter id class. If we want to support elements
	// such as `img` or namespaces, we'll need to add a case for that here.
	$selectors_scoped[] = '.' . $filter_id . trim( $selector_part );
	}
	$selector = implode( ', ', $selectors_scoped );

If you can get the filter id class on the lightbox wrapper and add the new lightbox selector to block.json, you should be able to get it to work.

Let me know if you need any help :)

[SantosGuillamot]

Issue explanation

I was reviewing this, and I believe we cannot get the filter id class in the lightbox container with the current implementation. The way I understood it:

• The duotone class is added via a render_block filter to the parent wrapper using this code.
• In the render_callback of the image, to add the lightbox, we are getting the <figure> from the block_content and adding it to the overlay <div>. Which results in the following format:

<figure>
   <img>
   <div class="overlay">
       <figure>
           <img>
       </figure>
   </div>
</figure>

The problem comes because the render_block filter runs after the render_callback. This means that:

• When the lightbox copies the <figure> to the overlay <div>, the duotone class doesn't exist yet.
• When the duotone filter adds the class, it only adds it to the parent <figure> and not to the one overlay <div>.

For this reason, I believe we cannot follow this approach unless we modify the duotone filter, which may result in other issues.

Potential solution

I was thinking about how to solve it, and maybe we can add the lightbox through a render_block_core/image filter as well, that runs after the default render_block. This way, it will support not only the changes made by the duotone but also other changes made through the render_block filter.

Apart from that, it would be a good first step to abstract the lightbox behavior of the image block, which is something worth exploring in the future.

Any opinions on this approach? Would it make sense?

If so, which is the best place to add this filter?

[cbravobernal]

Well, according to @mtias description in an issue. "Behaviors are meant to be reusable and composable interactive hooks that can be attached to blocks. "

In my opinion, for this and future behaviors, a filter per behavior should be the way to go, as it would be the "easiest" hay to apply it to different blocks, rather than upgrading each block. Ideally, behaviors should not be hard coded in the block folder.

Would be a good idea to have a proper package that could include all future behaviors?

[gziolo]

I wanted to echo what @c4rl0sbr4v0 said. It is a reasonable approach and aligns with how most of the block support features work today, including the duotone filter. You can use render_block with a different priority if you want to control the order of when processing for the lightbox and the duotone get applied. This way, you can have it ready for a more general usage that isn’t tied to the Image block only. It's also worth checking the existing implementations for block supports in https://github.com/WordPress/gutenberg/tree/trunk/lib/block-supports. Maybe some aspects can be abstracted using WP_Block_Supports::get_instance()->register().

By the way, there is also a proposal discussed for a new registerBlockSupport API on the client in #51005 that you can find relevant. If you would find a use case to shape it differently for the lightbox it would be great to report it back there.

[SantosGuillamot]

I opened this pull request to move the behaviors to block supports, which should solve the problem with the duotone filter. Should we close this pull request @artemiomorales ?

[artemiomorales]

@SantosGuillamot Sounds good! Let's continue discussion in the new pull request.