Skip to content

feat: box layout and resize + anchor observer primitives - #10558

Open
nwidynski wants to merge 6 commits into
adobe:mainfrom
nwidynski:feat-box-layout
Open

nwidynski wants to merge 6 commits into
adobe:mainfrom
nwidynski:feat-box-layout

Conversation

@nwidynski

Copy link
Copy Markdown
Contributor

Closes no issues directly, because this PR is intentionally limited to additions only. Migrating call sites to the new utilities and signatures is to be done in chore follow-ups, because it would otherwise be rather hard to review what changed here.

Disclaimer: This PR is tightly related to #10556! Unfortunately Github doesn't allow for Stacks to be built across forks, so we have to do with this :/

From a high-level, this PR fixes various issues with layout precision and lays the groundwork for an alternative to floating-ui. The primary file to look at is layout.ts, which features a short description.

Will close #7142, #10036, #10131, #9318 and more.

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices
  • I understand every change in this PR and can explain why it's there.
  • If AI-assisted, I followed our AI contribution guidance and pointed my assistant at CLAUDE.md.

📝 Test Instructions:

🧢 Your Project:

@nwidynski

nwidynski commented Sep 2, 2026 •

Copy link
Copy Markdown
Contributor Author

@LFDanLu This is the PR to look at for floating-ui, although it also addresses various other issues, like the ones coming from scrollbar-gutter: stable in Chrome. Please note that none of my PRs are AI assisted if not explicitly marked as such. I've just been working on these for quite some time so they land at once 😅

@reidbarber reidbarber left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome!

Comment thread packages/react-aria/src/utils/layout.ts
Comment thread packages/react-aria/src/utils/layout.ts Outdated
* Similar to `element.getBoundingClientRect()`, but intersected with all ancestors.
*/
public get visibleRect(): DOMRect {
throw new Error('Not implemented yet.');

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kinda. I don't have a great use case for it yet, besides window scrolling in virtualizer, so it just satisfies the type for now. IIRC it wasn't added in virtualizer because walking the tree would be too costly.

@github-actions github-actions Bot added the RAC label Sep 7, 2026
@nwidynski

Copy link
Copy Markdown
Contributor Author

@LFDanLu Is there anything left to do for me here? I saw the DNM branches got closed yesterday. I've been putting off work on test suites for this this PR and the scroll utilities PR, because they are quite a lot of work, and I haven't really gotten a definitive signal from you guys about whether to proceed here.

@LFDanLu

LFDanLu commented Sep 16, 2026

Copy link
Copy Markdown
Member

@nwidynski heya, the team tested on Monday and had some comments that came out of that as you have seen already on the other branches. I would echo Rob's sentiment here, where the stories you provided for this branch performed well, but the team wasn't quite sure the best way to validate that everything worked together/what other areas this fixed since we don't have visual regression tests nor stories for the linked issues setup already. We'll try and set that up next time

w/ regards to

I understand the dilemma from a reviewers perspective though. It's hard to tell which bugs this fixes without tests, but its also too much work to add tests for something you might not even want

100% agree, its definitely a ton of work to add all of this without knowing if it will be accepted and thank you for understanding our position where its hard to know whether we wanna accept something without that substantial work being done in the first place. Seeing as the goal of this is to eventually enable Navigation Component/Carousel and serve as an alternative for floating ui for overlay positioning, would it be helpful if we pointed these towards a feature branch instead of main so we can merge them together more easily and go from there? Trying to spitball how best to move forward here since this work will be long lived/have many moving parts and thus span multiple release cycles making it hard to merge things in confidently

@nwidynski

nwidynski commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor Author

@LFDanLu I've just decided to stop complaining about it and get it over with 😅 The scroll utils PR now has a full browser test suite, on which we can expand for scrollIntoView. I will see about getting something similar done for this PR once I find the time.

would it be helpful if we pointed these towards a feature branch instead of main so we can merge them together more easily and go from there?

I'm generally a bit hesitant towards that. Not because I don't agree with the logistics, but rather because I'm vary about stacking further PRs on top of something that I'm not sure will be accepted. Feature branches in my opinion only work if they receive the same review treatment as main, only with less requirement for polish. So I would basically have to be certain that once something lands on the feature branch, it at least is the right API/architecture, and will eventually go to main once polished & tested. Otherwise I risk building on top of unreviewed architecture decisions that render the whole feature branch useless in the end, even though there would have been mergable parts contained within. I hope that makes sense.

@nwidynski

Copy link
Copy Markdown
Contributor Author

@LFDanLu Okay, test suite for this PR is done also 👍 Migration will be done in follow-ups as mentioned, since they would blow up this PR. You can imagine DOMBox replacing literally any call site of element.getBoundingClientRect() or document.documentElement.clientWidth/clientHeight.

@devongovett

Copy link
Copy Markdown
Member

hey thanks for looking into improvements here! I have been thinking about two approaches that could get us out of the business of overlay positioning entirely:

  1. Adding an OverlayPositioner interface that could allow arbitrary custom positioning implementations, and implementing the current default on top of it. This could also allow other libraries like floating-ui to be used. Hacky prototype here
  2. Using CSS anchor positioning (possibly also implemented via the positioner interface). I had a branch with that as well, but there were a few browser bugs I was waiting to be fixed (which might be fixed by now).

I wondered what you thought of those options. Do you think it is worth us maintaining a completely independent implementation now that browsers have a built-in solution?

@nwidynski

nwidynski commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor Author

I have been thinking about two approaches that could get us out of the business of overlay positioning entirely

I believe there's been some misunderstanding. I think of @react-aria/overlays and @floating-ui as solutions to three distinct problems:

Problem @floating-ui @react-aria/overlays
A) Measuring layout ✅ platform 🟡 getRect, getContainerDimensions
B) Observing layout 🟡 autoUpdate 🟠 useResizeObserver, useViewportSize
C) Positioning layout ✅ computePosition 🟡 calculatePosition
✅ great 🟡 good, but… 🟠 not great

This PR solves A and B, while OverlayPositioner addresses C. An OverlayPositioner would simply build on DOMResizableBox and DOMAnchorBox, and calculatePosition would use DOMBox for measuring. I'm sure you are wondering why some of the cells aren't green. I'm happy to go into detail, but here's the short version.

A) @react-aria has several open issues with measuring edge cases (#10131, #8584, #10036, #7142, #10639, etc.), which we need to fix for both overlays and scrollIntoView.
B) @floating-ui's autoUpdate is expensive and less capable than native anchoring, while @react-aria's listeners miss all position changes that don't fire an event, e.g. layout shifts or transforms.
C) You know this one better than I do.


I wondered what you thought of those options. Do you think it is worth us maintaining a completely independent implementation now that browsers have a built-in solution?

As for my opinion on the two options, I'm a bit torn. On one hand, I think giving users options through abstracts like the recently added getTargetRect is awesome. On the other hand, I'm having a hard time imagining what a custom OverlayPositioner would look like. This isn't the same as something like a virtualizer layout, where each layout is a genuinely different behavior. @floating-ui, @react-aria/overlays and any other positioning library are all essentially polyfills for native CSS anchor positioning, so swapping one for another doesn't give you a different result, just a different implementation of the same thing.

So while I think Option 1 can make sense, I would keep it internal, and then migrate calculatePosition to Option 2 through this interface gradually, as you suggested. If an external use-case comes up later, we can always just export. If the question was whether or not I think overlays should be pure CSS eventually, I find that hard to answer. For agents yes, but humans I'm sure would appreciate a JS wrapper to hide the complexity of position-fallbacks.

@nwidynski

Copy link
Copy Markdown
Contributor Author

I had Claude put together a demo of the OverlayPositioner interface on top of the box primitives of this PR. One for native anchor positioning and one using the current positioning code.

@devongovett

devongovett commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

I think the question is if we did outsource this to the browser (anchor positioning) or a third party library (floating-ui), do we still need to maintain these low level utilities ourselves? I would assume floating-ui has all of these already so they wouldn't depend on anything we provide. And of course the browser would handle all this itself as well. In my opinion, ideally the bundle size for overlay positioning would go to ~zero because we swapped to the built in browser primitive. Whether we can do that in a minor or a major is an open question though.

@nwidynski

Copy link
Copy Markdown
Contributor Author

I think the question is if we did outsource this to the browser (anchor positioning) or a third party library (floating-ui), do we still need to maintain these low level utilities ourselves?

Yes, that's the right thought. Architecturally, there are only two use-cases in which measuring layout is a thing: anchor positioning and scroll(IntoView). So your question brings us back to container option and if-needed mode. WebKit will add container option in ~Safari 27.2. Once that ships, we likely still need the shim because the current trick for if-needed in scrollIntoViewport doesn't work reliably for async scroll, i.e. smooth scroll containers. Either way, this means we will have to maintain the shim at least until Safari 29.

So yes, I think the primitives here still make sense at this point. That being said though, with AnchorOverlayPositioner, our overlay code would drop down to ~300 lines + the primitives here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pinch zooming causes modal overlay to flicker

4 participants