Update acceptable anchor element algorithm
Hi, I updated the acceptable anchor element algorithm because I needed it for a project but I think it's good to merge upstream.
All test pass (one needed updating) and I added a new example to the home page.
I left a TODO which I already started to fix but decided it would be better to separate into another PR (pseudo-element anchors).
Closes #103
Deploy Preview for anchor-polyfill ready!
| Name | Link |
|---|---|
| Latest commit | db82d13bd717f1361ea97eb94d24f896d579c45a |
| Latest deploy log | https://app.netlify.com/sites/anchor-polyfill/deploys/6669d7954eb2a9000858ec18 |
| Deploy Preview | https://deploy-preview-195--anchor-polyfill.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Deploy Preview for anchor-position-wpt canceled.
| Name | Link |
|---|---|
| Latest commit | db82d13bd717f1361ea97eb94d24f896d579c45a |
| Latest deploy log | https://app.netlify.com/sites/anchor-position-wpt/deploys/6669d79573a96500081b59d6 |
@ayoreis Thanks for this PR! This is a huge help.
I haven't done a full review yet, but I did run it through the Web Platform Tests locally. This PR makes many more tests pass 🥳 , but there is a regression in anchor-name-inline-001.html.
It looks like originally all the anchor-sizes were resolving to 20, and in this PR, they now are all resolving to 30.
Per @xiaochengh in #103, "An element can use absolutely positioned anchors that come earlier in tree order". This is laid out more explicitly in the spec, with-
An element el is a acceptable anchor element for an absolutely positioned element query el if all of the following are true: ...
- If el has the same containing block as query el, then either el is not absolutely positioned, or el precedes query el in the tree order.
...
Thanks for looking at this @jamesnw.
Turns out I only ran the unit tests, I'm already working on fixing the e2e tests.
When running the Web Platform Tests (npm run test:wpt) I'm getting this error: Error: invariant: WPT_MANIFEST environment variable must be set. What do I need to set it to? Is there anything else to know?
Apologies for that- we're aware that WPT is one area that needs attention as we resume active development. What I'd recommend for right now-
- Clone and setup WPT for running locally.
- In the polyfill repo, run
npm run build:wpt - In the WPT repo, run
./wpt serve --inject-script=[path-to-polyfill-repo]/dist/css-anchor-positioning-wpt.umd.cjs - Open
http://web-platform.test:8000/tools/runner/index.html, and run the tests in the/css/css-anchor-positiondirectory. I usually limit to just the JavaScript Tests.
Note that there are roughly 6000 failures right now, so your goal isn't to get all WPT tests passing- that will be a larger effort.
Hi, sorry for taking so long. Quick update: I found the issue and started fixing it (Floating UI's platform.getOffsetParent does not work to get an element's containing block in all cases), I'll commit the fix tomorrow.
To check for regressions on a WPT run do you just use expectations metadata and then search for FAIL [expected PASS] on the output or is there a better way?
To check for regressions on a WPT run do you just use expectations metadata and then search for
FAIL [expected PASS]on the output or is there a better way?
I ran on main, and then on my branch, and exported the results as JSON from both. Then I did a diff between the two, which was incredibly noisy and pretty annoying. I would love to make that easier.
Hi, sorry I took so long again.
The problem was using the platform.getOffsetParent function to get the containing block (Floating UI also has a containingBlock function but oddly it performed worse). I created a new function and wrote the code for when the element is absolutely positioned, but still used platform.getOffsetParent for the other positions (spec says to use the element's formatting context).
Also:
- Applied @jgerigmeyer's reviews (thanks!)
- Added some comments with links to the spec
- Removed the special case for top layer elements
All previous Web Platform Tests pass + 46 new ones. I've been using this script I made to compare the results: https://github.com/ayoreis/wptr (It's been a great help, feel free to try it out).
Some end-to-end tests that are outdated compared with spec started failing, should I rewrite them?
@ayoreis We're going to merge this and fix up the tests on our end so we can cut a new release. Thanks again for your contribution!
Thanks!