Fix Lightbox trigger BUTTON visibility issue on an Image lightbox when Picture Element is enabled #1814

SohamPatel46 · 2025-01-21T13:13:33Z

Summary

Fixes #1805

Relevant technical choices

To fix the button visibility issue, we have conditionally added the custom CSS from the Modern Image Formats plugin when there is a Image block that has lightbox enabled.
Used filter - render_block_core/image to detect the above conditions. Ref.

codecov · 2025-01-21T13:28:39Z

Codecov Report

Attention: Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.

Project coverage is 59.53%. Comparing base (6803bc9) to head (876205e).
Report is 64 commits behind head on trunk.

Files with missing lines	Patch %	Lines
plugins/webp-uploads/picture-element.php	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk    #1814      +/-   ##
==========================================
- Coverage   59.58%   59.53%   -0.05%     
==========================================
  Files          84       84              
  Lines        6522     6527       +5     
==========================================
  Hits         3886     3886              
- Misses       2636     2641       +5

Flag	Coverage Δ
multisite	`59.53% <0.00%> (-0.05%)`	⬇️
single	`36.06% <0.00%> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

westonruter · 2025-01-21T17:51:34Z

plugins/webp-uploads/picture-element.php

+function webp_make_lightbox_trigger_button_visible() {
+    if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {
+		echo '<style>
+			.wp-lightbox-container > :hover {


Since we're specifically targeting the Picture element here, might as well make this specific to PICTURE. More importantly, this is missing the next-child + button selector:

Suggested change

.wp-lightbox-container > :hover {

.wp-lightbox-container picture:hover + button {

Used .wp-lightbox-container > picture + button as adding :hover to picture was not applying the CSS when tested. Let me know it the updated one looks good.

westonruter · 2025-01-21T17:52:22Z

plugins/webp-uploads/picture-element.php

+ */
+function webp_make_lightbox_trigger_button_visible() {
+    if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {
+		echo '<style>


I think this can rather be all put on one line.

westonruter · 2025-01-21T17:52:31Z

plugins/webp-uploads/picture-element.php

+ * @since 2.4.0
+ * 


The version we'll supply when creating the release:

Suggested change

* @since 2.4.0

*

* @since n.e.x.t

westonruter · 2025-01-21T17:52:57Z

plugins/webp-uploads/picture-element.php

+ * @since 2.4.0
+ * 
+ */
+function webp_make_lightbox_trigger_button_visible() {


Suggested change

function webp_make_lightbox_trigger_button_visible() {

function webp_make_lightbox_trigger_button_visible(): void {

westonruter · 2025-01-21T17:56:43Z

plugins/webp-uploads/picture-element.php

+		</style>';
+	}
+}
+add_action( 'wp_footer', 'webp_make_lightbox_trigger_button_visible' );


This action should be added conditionally based on whether the picture element is enabled (which we can detect via webp_uploads_is_picture_element_enabled()). See also:

performance/plugins/webp-uploads/hooks.php

Lines 725 to 728 in 53c410a

function webp_uploads_init(): void {

add_filter( 'wp_content_img_tag', webp_uploads_is_picture_element_enabled() ? 'webp_uploads_wrap_image_in_picture' : 'webp_uploads_filter_image_tag', 10, 3 );

}

add_action( 'init', 'webp_uploads_init' );

westonruter · 2025-01-21T17:58:27Z

plugins/webp-uploads/picture-element.php

+		echo '<style>
+			.wp-lightbox-container > :hover {
+				opacity: 1;
+			}
+		</style>';


I think this would be better all on one line:

Suggested change

echo '<style>

.wp-lightbox-container > :hover {

opacity: 1;

}

</style>';

echo '<style>.wp-lightbox-container picture:hover + button { opacity: 1 } </style>';

github-actions · 2025-01-22T15:00:08Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: SohamPatel46 <[email protected]>
Co-authored-by: westonruter <[email protected]>
Co-authored-by: felixarntz <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

felixarntz

Thanks @SohamPatel46, this looks on the right track. Two small points of feedback.

felixarntz · 2025-01-22T18:52:32Z

plugins/webp-uploads/picture-element.php

+/**
+ * Add an action to wp_footer conditionally for lighbox trigger button visibility.
+ *
+ * @since n.e.x.t
+ */
+function webp_add_footer_action_for_lightbox_button_visibility(): void {
+	if ( webp_uploads_is_picture_element_enabled() ) {
+		add_action( 'wp_footer', 'webp_make_lightbox_trigger_button_visible' );
+	}
+}
+add_action( 'init', 'webp_add_footer_action_for_lightbox_button_visibility' );


Do we really need two functions for this? I think it would be fine to unconditionally add the webp_make_lightbox_trigger_button_visible() function to wp_footer, and simply inside of it include an early return if ! webp_uploads_is_picture_element_enabled().

A related question is: Should this be in wp_footer or wp_head?

For classic themes this needs to be in wp_footer as otherwise we won't know if there is an Image block using a lightbox.

Makes sense, let's stick with wp_footer then. My other feedback is still applicable I think.

felixarntz · 2025-01-22T18:53:02Z

plugins/webp-uploads/picture-element.php

+ * @since n.e.x.t
+ */
+function webp_make_lightbox_trigger_button_visible(): void {
+	if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {


Please review the code linting failures, see https://github.com/WordPress/performance/actions/runs/12910976968/job/36002299388?pr=1814.

Suggested change

if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {

if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) === false ) {

westonruter · 2025-01-22T19:08:59Z

plugins/webp-uploads/picture-element.php

+ */
+function webp_make_lightbox_trigger_button_visible(): void {
+	if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {
+		echo '<style> .wp-lightbox-container > picture + button { opacity: 1 } </style>';


Without the :hover doesn't this mean it will always be shown?

westonruter · 2025-01-23T01:13:30Z

plugins/webp-uploads/picture-element.php

+ * @since n.e.x.t
+ */
+function webp_make_lightbox_trigger_button_visible(): void {
+	if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {


I don't know why, but the has_filter() check isn't working. This does though:

Suggested change

if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {

if ( false !== has_action( 'wp_footer', 'block_core_image_print_lightbox_overlay' ) ) {

westonruter · 2025-01-23T01:26:19Z

I'm seeing something problematic with the Interactivity API logic here. It's adding some inline styles to the BUTTON which is causing it to get positioned off-screen:

right: -629px;
top: -405px;

When Picture Element is not enabled, it's adding this style instead:

right: 16px;
top: 16px;

The culprit is display: contents. If I make these changes then it works as expected:

--- a/plugins/webp-uploads/picture-element.php
+++ b/plugins/webp-uploads/picture-element.php
@@ -162,7 +162,7 @@ function webp_uploads_wrap_image_in_picture( string $image, string $context, int
 	}
 
 	return sprintf(
-		'<picture class="%s" style="display: contents;">%s%s</picture>',
+		'<picture class="%s" style="/*display: contents;*/">%s%s</picture>',
 		esc_attr( 'wp-picture-' . $attachment_id ),
 		$picture_sources,
 		$image
@@ -175,8 +175,8 @@ function webp_uploads_wrap_image_in_picture( string $image, string $context, int
  * @since n.e.x.t
  */
 function webp_make_lightbox_trigger_button_visible(): void {
-	if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {
-		echo '<style> .wp-lightbox-container > picture + button { opacity: 1 } </style>';
+	if ( false !== has_action( 'wp_footer', 'block_core_image_print_lightbox_overlay' ) ) {
+		echo '<style> .wp-lightbox-container > picture:hover + button { opacity: 1 } </style>';
 	}
 }

I was not aware that the Image block was manually placing the button: https://github.com/WordPress/gutenberg/blob/b771bddb91ad74a7a03ee21b1ef311117efd815c/packages/block-library/src/image/view.js#L404-L438

And the addition of display:contents appears to be causing problems with that calculation. Also, the display:contents seems to be preventing the :hover pseudo-class from selecting the picture when it is hovered over.

But display:contents was added for a reason in #1307 to fix #1300.

Given all this, it doesn't seem like there is a quick fix which we can apply from our side.

westonruter · 2025-01-23T18:14:46Z

Thank you for being willing to tackle this!

Add Css to fix lightbox trigger button visibility

53c410a

westonruter added this to the webp-uploads 2.5.0 milestone Jan 21, 2025

westonruter added [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Bug An existing feature is broken labels Jan 21, 2025

westonruter requested changes Jan 21, 2025

View reviewed changes

SohamPatel46 added 4 commits January 22, 2025 17:32

Updated css, added action conditionally

6222df1

Add missing return statement for a function"

0c57433

Merge branch 'trunk' into fix/lightbox-trigger-button-visibility

d1cc1f5

Fix php sniffs

8982faf

SohamPatel46 marked this pull request as ready for review January 22, 2025 14:59

SohamPatel46 requested review from adamsilverstein and felixarntz as code owners January 22, 2025 14:59

Fix php sniffs

876205e

felixarntz reviewed Jan 22, 2025

View reviewed changes

westonruter reviewed Jan 22, 2025

View reviewed changes

westonruter reviewed Jan 23, 2025

View reviewed changes

westonruter mentioned this pull request Jan 23, 2025

Lightbox trigger BUTTON is not visible on an Image lightbox when Picture Element is enabled #1805

Open

westonruter removed this from the webp-uploads 2.5.0 milestone Jan 23, 2025

westonruter closed this Jan 23, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Lightbox trigger BUTTON visibility issue on an Image lightbox when Picture Element is enabled #1814

Fix Lightbox trigger BUTTON visibility issue on an Image lightbox when Picture Element is enabled #1814

SohamPatel46 commented Jan 21, 2025

codecov bot commented Jan 21, 2025 •

edited

Loading

westonruter Jan 21, 2025

SohamPatel46 Jan 22, 2025

westonruter Jan 21, 2025

westonruter Jan 21, 2025

westonruter Jan 21, 2025

westonruter Jan 21, 2025

westonruter Jan 21, 2025

github-actions bot commented Jan 22, 2025 •

edited

Loading

felixarntz left a comment

felixarntz Jan 22, 2025

westonruter Jan 22, 2025

felixarntz Jan 22, 2025

felixarntz Jan 22, 2025 •

edited

Loading

westonruter Jan 22, 2025

westonruter Jan 23, 2025

westonruter commented Jan 23, 2025

westonruter commented Jan 23, 2025

	.wp-lightbox-container > :hover {
	.wp-lightbox-container picture:hover + button {

	function webp_make_lightbox_trigger_button_visible() {
	function webp_make_lightbox_trigger_button_visible(): void {

	function webp_uploads_init(): void {
	add_filter( 'wp_content_img_tag', webp_uploads_is_picture_element_enabled() ? 'webp_uploads_wrap_image_in_picture' : 'webp_uploads_filter_image_tag', 10, 3 );
	}
	add_action( 'init', 'webp_uploads_init' );

	if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {
	if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) === false ) {

	if ( has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) ) {
	if ( false !== has_action( 'wp_footer', 'block_core_image_print_lightbox_overlay' ) ) {

Fix Lightbox trigger BUTTON visibility issue on an Image lightbox when Picture Element is enabled #1814

Fix Lightbox trigger BUTTON visibility issue on an Image lightbox when Picture Element is enabled #1814

Conversation

SohamPatel46 commented Jan 21, 2025

Summary

Relevant technical choices

codecov bot commented Jan 21, 2025 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 22, 2025 • edited Loading

felixarntz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

felixarntz Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter commented Jan 23, 2025

westonruter commented Jan 23, 2025

codecov bot commented Jan 21, 2025 •

edited

Loading

github-actions bot commented Jan 22, 2025 •

edited

Loading

felixarntz Jan 22, 2025 •

edited

Loading