[#279] Fix position-try demos and polyfill logic#424
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.
|
| const anchorPosition: AnchorPosition = {}; | ||
| if (order) { | ||
| anchorPosition.order = order; | ||
| } |
There was a problem hiding this comment.
Claude's explanation of this change:
In the polyfill, a comma-separated selector list that uses position-try-fallbacks (e.g. #a, #b { position-try-fallbacks: flip-block, … }) caused the second target to be positioned using the first target's fallbacks. Visibly, in Firefox the second target stayed static (un-anchored) and targets could revert to a non-anchored position when a fallback couldn't be placed.
Cause
In parsePositionFallbacks, the anchorPosition object was declared outside the selectors.forEach loop, so it was shared across every selector in the list. Because try-tactic fallbacks are keyed per selector (${selector}-${tactics}), each selector's fallbacks accumulated into that one shared object — and since validPositions[selectorA] and validPositions[selectorB] both referenced it, the second target's fallback list began with the first target's fallbacks (which resolve against the wrong anchor). @position-try (at-rule) fallbacks were unaffected because their names are shared across selectors.
Fix
Give each selector its own anchorPosition, and separate the two concerns that were conflated under a single dedup guard:
- Add a fallback to this selector's position — now tracked per selector.
- Inject the generated
@position-tryrule into the stylesheet — still deduped once per stylesheet.
Tests
Added a regression unit test asserting that each selector in a shared rule gets only its own try-tactic fallback. Existing unit (186) and e2e (37) suites pass.
| CSS.registerProperty({ | ||
| name: customProperty, | ||
| syntax: '*', | ||
| inherits: false, | ||
| }); |
There was a problem hiding this comment.
TIL about CSS.registerProperty 🎉
1 participant
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.