[#279] Fix position-try demos and polyfill logic#424
Draft
jgerigmeyer wants to merge 5 commits into
Draft
Conversation
✅ Deploy Preview for anchor-polyfill ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for anchor-position-wpt canceled.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #279
Background
The
position-trydemos didn't update on scroll, and were broken in native browsers too, not just under the polyfill. Investigating in real native Chrome (150) confirmed two distinct problems:Demo authoring bug (affects native). Per spec,
position-tryoptions are evaluated by checking whether the target overflows its inset-modified containing block — not the scrollport of an ancestor scroll container. Every demo target's containing block was its.scroll-container(position: relative), which also contains the anchor. So scrolling moved the anchor and target together and the target never overflowed its containing block → fallbacks never triggered, natively or otherwise. The polyfill only appeared to work because it measured overflow against the scrollport, diverging from native.Polyfill inheritance leak. Properties shifted into custom properties (
height, insets, margins,anchor-name, …) are all non-inherited in CSS, but custom properties inherit by default. Soheight: 400pxon a.scroll-containerleaked through--height-<uuid>into every descendant target and got stamped into every generated fallback.Changes
Demos (
public/position-try*.css): make the position-try scroll containersposition: staticso each target's containing block is promoted to the surrounding.demo-elementswrapper (an ancestor outside the scroller). Now scrolling pushes the target to overflow and fallbacks trigger — natively and under the polyfill. Addedoverflow: clipon the wrapper so a target whose anchor has scrolled out of view doesn't float over other content, and gave the combined demo's anchor an offset so its base position fits at rest. Removed the "this example is broken" warning inindex.html.Non-inherited custom properties (
src/cascade.ts,src/dom.ts): register the shifted custom properties withCSS.registerProperty({ syntax: '*', inherits: false })so a value set on an ancestor is no longer read back as if it were set directly on a descendant. Registration is global and runs once; it no-ops whereCSS.registerPropertyis unavailable. Updated the now-inaccurate inheritance comments.Overflow basis (
src/polyfill.ts): rewrotecheckOverflowto measure the target's margin-box against its containing block (offsetParent) usinggetBoundingClientRect, matching the spec/native behavior instead of floating-ui's scrollport-baseddetectOverflow. Both rects are read in viewport coordinates, so the page's scroll offset cancels out (the previous standalonedetectOverflowcall produced wildly wrong values once the containing block was no longer a near-viewport scroll container). Dropped the now-unuseddetectOverflow/MiddlewareStateimports andplatformWithCache.Test (
tests/e2e/polyfill.test.ts): the target'soffsetParentis now the (non-scrollable) wrapper, so the@position-fallbacktest scrolls.scroll-containerdirectly and captures the pre-polyfill position rather than hardcoding it.Behavior change to flag for review
checkOverflownow matches native semantics (overflow vs. containing block). This preserves existing behavior for the common cases — an absolutely-positioned target whose containing block is a scroll container still flips against that scroller's visible box, and a fixed target flips against the viewport — so nothing in the suite regressed. But it is a change to core polyfill logic and worth a careful look.Verification
height: 400pxleaks into generated fallbacks; no console errors.