Takshil Kunadia

Results 13 comments of 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!