Pseudo-element anchors #213

ayoreis · 2024-07-02T20:12:16Z

TODO:

Fix the "Positioning with anchor() [passed through multiple CSS custom properties]" example. customPropsMapping seems not to get populated.
Update tests to expect pseudo elements.
Get WPT passing for pseudo-element anchors. The polyfill is working (if I manually check the conditions, eg. anchored1.offsetTop === 100) but WPT reports 0, no tests went down.

~~Performance does not seem worse (tested with console.time consistently at ~110ms).~~

Closes #208.

netlify · 2024-07-02T20:12:32Z

✅ Deploy Preview for anchor-position-wpt canceled.

Name	Link
🔨 Latest commit	`1483a76`
🔍 Latest deploy log	https://app.netlify.com/sites/anchor-position-wpt/deploys/669c1677c064780008ba56de

netlify · 2024-07-02T20:12:33Z

✅ Deploy Preview for anchor-polyfill ready!

Name	Link
🔨 Latest commit	`1483a76`
🔍 Latest deploy log	https://app.netlify.com/sites/anchor-polyfill/deploys/669c16770044a900089e85d6
😎 Deploy Preview	https://deploy-preview-213--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.

ayoreis · 2024-07-15T15:48:59Z

I updated the tests and fixed @position-fallback. Also now instead of inserting the pseudo-element's content as text into the fake pseudo-element, it add it to the ::before of the fake pseudo-element supporting all types (eg. images).

Currently all these changes add about 15ms to the polyfill (on the homepage, on my machine). It seem that the increase is only because of enabling parseRulePrelude for csstree, so it won't increase much with the amount of pseudo-element anchors, so that amount seems fine.

I looked into WPT, the problem is that --inject-script is sync and we have async code, so WPT runs before everything has been polyfilled, I think window.CHECK_LAYOUT_DELAY in src/index-wpt.ts should fix this but I can't find any docs on it to continue debugging.

One unrelated WPT test started passing with these changes (/css/css-anchor-position/anchor-query-custom-property-registration.html).

jamesnw · 2024-07-15T16:55:07Z

I looked into WPT, the problem is that --inject-script is sync and we have async code, so WPT runs before everything has been polyfilled, I think window.CHECK_LAYOUT_DELAY in src/index-wpt.ts should fix this but I can't find any docs on it to continue debugging.

window.CHECK_LAYOUT_DELAY was added to the WPT in order to make this polyfill testable. It is currently used in checkLayoutForAnchorPos to add a delay of 3 animation frames before checking layout. See this comment for more info.

The pseudoelement tests are not using checkLayoutForAnchorPos, and so I think we'll need another strategy. Perhaps we add a function to WPT that we can use to delay other tests that aren't using checkLayoutForAnchorPos? I'm guessing this is also the case for other test files as well.

ayoreis · 2024-07-15T17:23:50Z

I'm not familiar with the WPT codebase, but could <script type="module"> injected scripts be implemented? If yes, I might be interested in working on it with some guidance.

jamesnw · 2024-07-15T17:34:18Z

I'm not familiar with the WPT codebase, but could <script type="module"> injected scripts be implemented? If yes, I might be interested in working on it with some guidance.

Yeah- there are some <script type="module"> examples on other anchor position tests. I think you can run ./wpt serve --inject-script=[polyfillpath] and run it locally (note serve rather than run in this context. You can navigate to http://web-platform.test:8000/css/css-anchor-position/pseudo-element-anchor.html for example and that should have the polyfill loaded.

ayoreis · 2024-07-15T19:18:33Z

I think I didn't explain well. What I meant is: Is it possible to implement support for injecting type="module" scripts that run before the test, for example this is the resulting test with --inject-script:

<!DOCTYPE html>
<script>
await console.log('Hello') // throws

// Remove the injected script tag from the DOM.
document.currentScript.remove();
</script>

<title>Test</title>

<div id="element">Test</div>

<script>
  test(() => {
    assert_equals(element.textContent, 'Test');
  }, 'Test');
</script>

And with something like --inject-script-module, if it could be implemented:

<!DOCTYPE html>
<title>Test</title>

<div id="element">Test</div>

<script type="module">
await console.log('Hello') 

// Remove the injected script tag from the DOM.
document.currentScript.remove();
</script>

<script>
  test(() => {
    assert_equals(element.textContent, 'Test');
  }, 'Test');
</script>

This would solve our use case.

A issue with this idea is that all tests would have to be converted to <script type="module">s (because type="module" scripts defer), a workaround could be for the test function to wrap the callback in an addEventListener('load'), but I don't know if that would cover all cases.

jamesnw · 2024-07-15T20:21:49Z

I think I didn't explain well.

No- you explained well, on my second reading I realized I had misunderstood, but you got your response up first.

To boil down the problem, the tests are running before the polyfill is complete. Ideally, we would add some delays for testing the polyfill that wouldn't impact non-polyfill tests. I don't think that adding an injected module script would be simple.

We do have a few tools at our disposal- pseudo-element-anchor.html works if you use promise_test and waitForAtLeastOneFrame()-

<script>
  promise_test(async () => {
    await waitForAtLeastOneFrame();
    assert_equals(anchored1.offsetLeft, 100);
    assert_equals(anchored1.offsetTop, 100);
  }, "::before as anchor");
  promise_test(async () => {
    await waitForAtLeastOneFrame();
    assert_equals(anchored2.offsetLeft, 100);
    assert_equals(anchored2.offsetTop, 100);
  }, "::after as anchor");
</script>

But that also impacts non-polyfilled tests. @jgerigmeyer Do you have any thoughts on an analog to the window.checkLayoutForAnchorPos might be? I'm wondering about a waitForAtLeastOneFrame wrapper that only waits if CHECK_LAYOUT_DELAY is true?

jgerigmeyer · 2024-07-15T21:12:35Z

@jgerigmeyer Do you have any thoughts on an analog to the window.checkLayoutForAnchorPos might be? I'm wondering about a waitForAtLeastOneFrame wrapper that only waits if CHECK_LAYOUT_DELAY is true?

@jamesnw Yeah, when we landed web-platform-tests/wpt#38442 we knew that there were a few WPTs that called test() directly, and it didn't fix those cases. We might want to check with mfreed7 to see what the WPT folks would be open to, but I could see adding something similar to checkLayoutForAnchorPos that would replace test() for anchor-position tests only.

jamesnw · 2024-07-16T14:38:10Z

I opened a PR on oddbird:wpt https://github.com/oddbird/wpt/pull/3/files that adds a test function which waits for the polyfill to run. It makes pseudo-element-anchor.html pass, but doesn't work on pseudo-element-anchor-dynamic.html because dynamic changes aren't yet supported.

If this seems like it will fix this, I'll check with mfreed7 and work on upstreaming.

jamesnw

I think this is looking pretty good!

At some point, could you also open issues for the things we are not covering here- other pseudo elements like the file upload input button, or scrolling?

public/anchor-pseudo-element.css

src/parse.ts

src/validate.ts

jgerigmeyer

A few questions and minor suggestions, but this is looking good!

index.html

src/parse.ts

src/validate.ts

jgerigmeyer

This looks good to me! 🚀

jgerigmeyer · 2024-07-22T18:46:17Z

@ayoreis I'm going to go ahead and merge this -- thanks!

I think the only outstanding somewhat-related item I see is this, but it's not urgent/blocking:

At some point, could you also open issues for the things we are not covering here- other pseudo elements like the file upload input button, or scrolling?

Pseudo-element anchors

cc71cfd

ayoreis force-pushed the pseudo-element-anchors branch from b6c871c to cc71cfd Compare July 3, 2024 19:00

ayoreis added 2 commits July 3, 2024 20:01

Use Selector everywhere, clean up

0e41ebc

Update tests, fix @position-fallback, support all content types

9fd7440

jamesnw reviewed Jul 16, 2024

View reviewed changes

public/anchor-pseudo-element.css Outdated Show resolved Hide resolved

public/anchor-pseudo-element.css Outdated Show resolved Hide resolved

src/parse.ts Show resolved Hide resolved

src/validate.ts Outdated Show resolved Hide resolved

src/validate.ts Outdated Show resolved Hide resolved

jgerigmeyer reviewed Jul 17, 2024

View reviewed changes

ayoreis added 3 commits July 18, 2024 15:21

Merge branch 'oddbird:main' into pseudo-element-anchors

632511f

Merge branch 'oddbird:main' into pseudo-element-anchors

b641a96

Changes based on reviews, styles, docs, small bugs

1483a76

ayoreis marked this pull request as ready for review July 20, 2024 19:56

jgerigmeyer approved these changes Jul 22, 2024

View reviewed changes

jgerigmeyer requested a review from jamesnw July 22, 2024 16:53

jgerigmeyer merged commit a1deb34 into oddbird:main Jul 22, 2024
10 checks passed

jgerigmeyer mentioned this pull request Jul 22, 2024

Add anchor-scope support #217

Merged

ayoreis mentioned this pull request Jul 23, 2024

Support all part-like pseudo-elements. #219

Open

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Pseudo-element anchors #213

Pseudo-element anchors #213

ayoreis commented Jul 2, 2024 •

edited

Loading

netlify bot commented Jul 2, 2024 •

edited

Loading

netlify bot commented Jul 2, 2024 •

edited

Loading

ayoreis commented Jul 15, 2024

jamesnw commented Jul 15, 2024

ayoreis commented Jul 15, 2024

jamesnw commented Jul 15, 2024

ayoreis commented Jul 15, 2024

jamesnw commented Jul 15, 2024

jgerigmeyer commented Jul 15, 2024 •

edited

Loading

jamesnw commented Jul 16, 2024 •

edited

Loading

jamesnw left a comment

jgerigmeyer left a comment

jgerigmeyer left a comment

jgerigmeyer commented Jul 22, 2024

Pseudo-element anchors #213

Pseudo-element anchors #213

Conversation

ayoreis commented Jul 2, 2024 • edited Loading

netlify bot commented Jul 2, 2024 • edited Loading

✅ Deploy Preview for anchor-position-wpt canceled.

netlify bot commented Jul 2, 2024 • edited Loading

✅ Deploy Preview for anchor-polyfill ready!

ayoreis commented Jul 15, 2024

jamesnw commented Jul 15, 2024

ayoreis commented Jul 15, 2024

jamesnw commented Jul 15, 2024

ayoreis commented Jul 15, 2024

jamesnw commented Jul 15, 2024

jgerigmeyer commented Jul 15, 2024 • edited Loading

jamesnw commented Jul 16, 2024 • edited Loading

jamesnw left a comment

Choose a reason for hiding this comment

jgerigmeyer left a comment

Choose a reason for hiding this comment

jgerigmeyer left a comment

Choose a reason for hiding this comment

jgerigmeyer commented Jul 22, 2024

ayoreis commented Jul 2, 2024 •

edited

Loading

netlify bot commented Jul 2, 2024 •

edited

Loading

netlify bot commented Jul 2, 2024 •

edited

Loading

jgerigmeyer commented Jul 15, 2024 •

edited

Loading

jamesnw commented Jul 16, 2024 •

edited

Loading