fix: audit incorrectly flagging images as above the fold (#12993)

[Kynson]

Changes

• Fixes Toolbar audit incorrectly flag images as above the fold #12993
• Previously, the logic for calculating whether the image is above (below) the flow depends on window.scrollY
• This will not work when the body element is not scrollable/ not the scrolling container (i.e. window.scrollY always 0)
• The fix calculates the actual y position by adding offsetTop value of the target element itself and all the offest parent(s)
• The fix is proposed by @hfournier in Toolbar audit incorrectly flagging images as above the fold #10891

Testing

No tests are added (manual testing is done locally), as I am not familiar with playwright and the fix is trivial. Please help me to add some tests if it is necessary.

Docs

Docs update should not be necessary.

[changeset-bot]

🦋 Changeset detected

Latest commit: f657b1f

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[codspeed-hq]

CodSpeed Performance Report

Merging #12998 will not alter performance

_{Comparing Kynson:main (f657b1f) with main (429aa75)}

Summary

✅ 6 untouched benchmarks

[ascorbic]

Does this mean that the other fix proposed in #10891 and implemented in #11617 didn't work?

[Kynson]

As far as I can tell, the fix implemented in #11617 can't handle all situations (as mentioned in my original issue).

However, there were two different fix proposed in #10891:

1. The one implemented in fix: audit incorrectly flagging images as above the fold (#10891) #11617 (depending on window.scrollY)
2. The one submitted in this PR

While the second fix is more complicated than the fix in #11617, but according to my testing, it can also handle the situation which the image is a descendent of a scrolling container other than body

[ascorbic]

It would be fantastic if you could get a test in for this. I think it's clear that there are edge cases which the other PR didn't catch, which is why an e2e test would be just what's needed. There is already a suite testing the audits, so you could add a case in there:

https://github.com/withastro/astro/blob/main/packages/astro/e2e/dev-toolbar-audits.test.js

The fixture is here, where you could add something like the page you manually tested with:

https://github.com/withastro/astro/blob/main/packages/astro/e2e/fixtures/dev-toolbar

[Kynson]

Thanks for the links! I am happy to write some e2e tests if the team doesn't mind I might need some time to get familiar with playwright.

[ascorbic]

No rush. Playwright is pretty nice to work with, so it shouldn't be too hard to get started though.

[Kynson]

I have added some tests:

• Image in body that is not scrollable (for the fix in this PR)
• Image in relative container (ensure it still fixes Toolbar audit incorrectly flagging images as above the fold #10891)
• Image in absolute container

Please tell me if more edge cases are needed.