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

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

Open
westonruter opened this issue Jan 17, 2025 · 4 comments
Labels
blocked [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Bug An existing feature is broken

Comments

@westonruter
Copy link
Member

Bug Description

I just found that Modern Image Formats prevents that button from ever being visible on hover when Picture Element is enabled. This is because of this CSS rule (source):

.wp-lightbox-container img:hover+button {
    opacity: 1;
}

That rule would not be necessary if the BUTTON.lightbox-trigger wrapped the IMG, since hovering over the button could always then cause the SVG to be revealed:

.wp-lightbox-container button.lightbox-trigger:hover > SVG {
    opacity: 1;
}

Originally discovered in WordPress/gutenberg#68726 (comment).

Steps to reproduce

  1. Activate Modern Image Formats plugin
  2. Enable Picture Element in the plugin settings
  3. Upload an image
  4. Add the image to a new Image block and enable lightbox
  5. Hover over the Image block with your mouse

The lightbox trigger button is not visible.

Screenshots

The lightbox trigger button is supposed to appear in the top-right corner of the image:

Image

@westonruter 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 17, 2025
@github-project-automation github-project-automation bot moved this to Not Started/Backlog 📆 in WP Performance 2024 Jan 17, 2025
@SohamPatel46
Copy link
Contributor

Hello, I would like to work on this and fix it. Thanks !

@westonruter
Copy link
Member Author

I suppose there are two ways this could go. One, Gutenberg's CSS selector could be updated to remove the img, something like this:

.wp-lightbox-container > :hover+button {
    opacity: 1;
}

Otherwise, maybe the better course for now would be to conditionally add that CSS from the Modern Image Formats plugin when the Picture Element setting is enabled and there is a Image block that has lightbox enabled.

For example, at wp_footer you could check if has_filter( 'render_block_core/image', 'block_core_image_render_lightbox' ) and then print out that additional CSS if it is not false. You can see this filter is added when the lightbox appears on the page: https://github.com/WordPress/wordpress-develop/blob/7fe8f1cc6f2722d773c738bbe0811bef2a07756e/src/wp-includes/blocks/image.php#L67-L83

@westonruter
Copy link
Member Author

westonruter commented Jan 23, 2025

It appears we can't fix this easily. See #1814 (comment):

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.

@adamsilverstein
Copy link
Member

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

Interesting, maybe we can rethink how the button is placed in Gutenberg? It feels a bit fragile as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked [Plugin] Modern Image Formats Issues for the Modern Image Formats plugin (formerly WebP Uploads) [Type] Bug An existing feature is broken
Projects
Status: Not Started/Backlog 📆
3 participants