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

Compatibility issue after WordPress 6.4 with the new lightbox functionality #758

Closed
ionikol opened this issue Nov 16, 2023 · 12 comments · May be fixed by #762
Closed

Compatibility issue after WordPress 6.4 with the new lightbox functionality #758

ionikol opened this issue Nov 16, 2023 · 12 comments · May be fixed by #762
Labels
can't reproduce effort [XS] < 1 day of estimated development time priority: high Issues which should be resolved as quickly as possible severity: moderate type: bug

Comments

@ionikol
Copy link

ionikol commented Nov 16, 2023

Describe the bug
When the "Display images in WebP format on the site” option is turned on as tags and the new lightbox functionality on WordPress 6.4 is closed the site’s scroll gets stuck/broken.

To Reproduce
Steps to reproduce the behavior:

  1. Update WordPress 6.4 to have the new lightbox feature
  2. Enable lightbox on an image
  3. Have the "Display images in WebP format on the site" turned on as tags
  4. Click on image on frontend to open lighbox
  5. Click X in top right of image to close lightbox
  6. Can't scroll up/down even though lightbox closed

Additional context
Tickets from HS and WordPress.org
https://secure.helpscout.net/conversation/2416490954/453777/
https://wordpress.org/support/topic/imagify-conflicts-with-wordpress-6-4s-new-lightbox-functionality/

Acceptance Criteria (for WP Media team use only)
Clear instructions for developers, to be added before the grooming

@piotrbak piotrbak added needs: grooming priority: high Issues which should be resolved as quickly as possible type: bug severity: moderate labels Nov 16, 2023
@piotrbak piotrbak added this to the 2.1.4 milestone Nov 16, 2023
@piotrbak
Copy link

It can be seen here:
https://rocketlabsqa.ovh/testing-image-lightbox/

@CrochetFeve0251 CrochetFeve0251 self-assigned this Nov 20, 2023
@CrochetFeve0251
Copy link
Contributor

CrochetFeve0251 commented Nov 21, 2023

Root cause

The root cause is that we are adding the attributes on both the img tag and picture one.

Scope a solution

A simple solution would be to filter out WordPress attributes for the picture tag in classes/Webp/Picture/Display.php:

		$p_attributes = array_filter($attributes, function ($attribute) {
			return ! str_contains($attribute, 'data-wp');
		}, ARRAY_FILTER_USE_KEY);

		$output = '<picture' . $this->build_attributes( $p_attributes ) . ">\n";

Estimate effort

Effort XS

@jeawhanlee
Copy link
Contributor

Looks good to me.

@jeawhanlee jeawhanlee added effort [XS] < 1 day of estimated development time and removed needs: grooming labels Nov 21, 2023
@CrochetFeve0251
Copy link
Contributor

@MathieuLamiot @engahmeds3ed is checking how the interactivity package is working inside Guttenberg as adding a callback to the lightbox doesn't seems to be attached to the context

@CrochetFeve0251 CrochetFeve0251 removed their assignment Dec 15, 2023
@wordpressfan
Copy link
Contributor

There is a progress there in Gutenberg for this issue, they opened a new PR to solve that:
WordPress/gutenberg#57089

I'll test it with this PR but I believe it'd work, hope they release it soon.

@MathieuLamiot
Copy link
Contributor

@piotrbak Gutenberg 17.4 is not released yet (RC01 ongoing). Should this issue be delayed to Imagify 2.1.5 then?

@piotrbak
Copy link

piotrbak commented Jan 2, 2024

@MathieuLamiot @CrochetFeve0251 Just to confirm, when Gutenberg releases the enhancement, will it fix the issue automatically or it'll allow us to fix it on our end?

@MathieuLamiot
Copy link
Contributor

@wordpressfan
Copy link
Contributor

We still need this PR to be merged to add the attributes to pictures.
The Gutenberg PR that will be shipped with v17.4 will fix the console error that Vasilis mentioned here:
#762 (comment)

@remyperona remyperona removed this from the 2.1.4 milestone Mar 6, 2024
@rfischmann
Copy link

So this is still an issue today?

@soloman981
Copy link

So this is still an issue today?

Yes it is :)

@piotrbak piotrbak added this to the 2.2.3 milestone Aug 8, 2024
@wordpressfan
Copy link
Contributor

@rfischmann @soloman981
I can't replicate it from my side, can you please share the WP and imagify versions being used from your side to try replicating it?
Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
can't reproduce effort [XS] < 1 day of estimated development time priority: high Issues which should be resolved as quickly as possible severity: moderate type: bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants