Takshil Kunadia
Takshil Kunadia
Tried to reproduce this issue with PHP 8.2 but this issue doesn't seem to be present anymore. Confirmed with @milindmore22 as well.
Should we add the same checks here as well? https://github.com/ampproject/amp-wp/blob/79b1b7a5b00e2d4e5872b26c5f96469b244dca96/includes/class-amp-theme-support.php#L1404-L1414 https://github.com/ampproject/amp-wp/blob/79b1b7a5b00e2d4e5872b26c5f96469b244dca96/includes/class-amp-theme-support.php#L1426-L1435
Thanks! @Mamaduka . I felt this PR provided a more complete solution. But do let me know if I should close it in favour of https://github.com/WordPress/gutenberg/pull/67554
> If there is a 200 ms timeout to start preloading would that have prevented the issue Updated code on the [PR](https://github.com/WordPress/gutenberg/pull/61107) to have a 200ms delay before prefetching ✅
Thanks @artemiomorales ! for the context and for looping the stakeholders in the discussion! I've updated the code to reflect the direction discussed in the issue. Happy to adjust further...
Thanks! @westonruter for looking into this 🙇🏻♂️ and yes good catch! it makes sense to have responsive images for lightbox images to avoid unnecessarily large downloads, especially on smaller viewports....
Hi @westonruter I've implemented the refactor as per your last comment. Since the PR has been approved, just wanted to check if it’s good to merge ✅
Hi @westonruter, just following up on this PR since it’s been approved. Please let me know if there's anything else needed from my side to get it merged. Thanks ☺️!
Rebased to keep this branch up-to-date with `trunk` and resolve conflicts ✅
Hey @luisherranz This PR has already been reviewed and approved, so just wondering what’s next in the release process. Would love to see it land soon!