Skip to content

fix: measure overlay natural width before positioning#10295

Open
AKnassa wants to merge 1 commit into
adobe:mainfrom
AKnassa:fix/overlay-natural-width-measurement
Open

fix: measure overlay natural width before positioning#10295
AKnassa wants to merge 1 commit into
adobe:mainfrom
AKnassa:fix/overlay-natural-width-measurement

Conversation

@AKnassa

@AKnassa AKnassa commented Jul 3, 2026

Copy link
Copy Markdown

Closes #10050
Closes #7017

What this does

Menus, popovers, and tooltips that open near the right edge of the page no longer open too narrow and visibly stretch wider step by step. They now open at their correct width in one clean move.

Why

When an overlay is repositioned, its width was measured while the previous position was still applied to it. Being pinned near the right page edge, the browser only gave it the leftover space up to that edge, so the measured width came out too small. Each reposition pass then let it grow only a little further, which users saw as a jittery widening animation (and, for tooltips, as dozens of reposition passes with ResizeObserver loop errors — #7017).

The fix mirrors what useOverlayPosition already does vertically before measuring (top/bottom/maxHeight reset): the horizontal insets are now reset too, so the overlay lays out at its natural width before its position is calculated.

overlay.style.left = '0px';
overlay.style.right = '';

Two details matter here, and both are covered by tests:

  • right must be cleared together with left. Physical-left placements (left ..., start in LTR, end in RTL, flipped submenus) position via a right inset. Setting left: 0px while a stale right remains stretches the overlay between both insets and over-measures it, causing spurious flips. This is the difference from the earlier attempt in Fix resize loop issue #8164, which set only left and was never merged.
  • The reset is intentionally outside the !maxHeight guard. Width measurement is unrelated to the user-provided maxHeight prop, so overlays that pass one get the fix too.

Before / after

Before (menu at the right page edge, item descriptions render after the initial positioning pass):
issue-10050-before-fix

After:
issue-10050-after-fix

What changed

  • packages/react-aria/src/overlays/useOverlayPosition.ts — reset left/right before the measurement pass (8 lines).
  • packages/react-aria/test/overlays/useOverlayPosition.test.tsx — 5 new tests with a shrink-to-fit offsetWidth mock that models how a stale inset clamps an auto-width absolutely-positioned element: single-pass convergence when content grows, the stale-right stretch guard, maxHeight-prop independence, RTL (end placements), and placement-change inset cleanup.
  • packages/react-aria/stories/overlays/UseOverlayPosition.stories.tsx — two repro stories (bottom end and start top) with a right-edge trigger whose content grows 1.5s after opening.

Notes for reviewers

  • Expected behavior change: flip decisions near viewport edges become accurate for left/right placements, because the flip check now sees the true natural width instead of a stale-clamped one. Overlays that previously stayed squeezed on one side may now correctly flip. Chromatic diffs on edge-adjacent submenus/tooltips would be the fix working, not regressions — a Chromatic run on this PR would be great (this is what was requested on Fix resize loop issue #8164 before it was abandoned).
  • Regression surface run locally in StrictMode, all green (682 tests / 24 suites): react-aria overlays, RAC Popover/Menu/Tooltip/Select/ComboBox, v3 menu incl. SubmenuTrigger, v3 tooltip, S2 Menu/Picker/Combobox. Select/ComboBox --trigger-width matching is unaffected (computed from the trigger, not the overlay).
  • One thing intentionally left alone: the early return if (!position.position) return; between the measurement reset and the style re-apply is currently unreachable (calculatePosition always returns a position), but if an exception ever escaped calculatePosition, the overlay would be stranded at the reset position. Flagging rather than fixing to keep this diff minimal — happy to harden it here if preferred.

✅ 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

📝 Test Instructions:

  1. yarn jest packages/react-aria/test/overlays/useOverlayPosition.test.tsx — includes the new auto-width overlay (shrink-to-fit) suite.
  2. In the dev storybook (yarn start), open UseOverlayPosition → growing content at right page edge (Spectrum 2 ActionMenu renders too narrow for descriptions on page right #10050), click the trigger, and wait ~1.5s for the descriptions to render. The menu should snap to its full width in a single move with no incremental widening. Repeat with the start top variant and with an RTL locale.
  3. For Tooltip causing ResizeObserver issue #7017: open the same story and confirm the overlay settles in one reposition with no ResizeObserver loop completed with undelivered notifications console errors.

🧢 Your Project:

Reset the overlay's left/right insets before measuring in
useOverlayPosition, mirroring the existing top/bottom reset. A stale
inset from the previous positioning pass clamps an auto-width overlay's
shrink-to-fit width, so overlays near the right page edge opened too
narrow and visibly widened step by step as each ResizeObserver pass
grew them a little further. Both insets must be reset together, since
physical-left placements position via a right inset and a leftover
value would stretch the overlay during measurement instead.

Closes adobe#10050, closes adobe#7017
@AKnassa AKnassa marked this pull request as ready for review July 3, 2026 20:35
Comment on lines +355 to +356
// a single pass (360 + 100 - 300). With a stale left of 400px the measurement
// is clamped to 100px and the overlay only creeps toward its final position.

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.

Suggested change
// a single pass (360 + 100 - 300). With a stale left of 400px the measurement
// is clamped to 100px and the overlay only creeps toward its final position.
// a single pass (360 + 100 - 300).

What did that sentence mean?

// Pin the trigger to the right viewport edge regardless of story decorators,
// so the overlay has to fit its width against the page boundary (issue #10050).
<div style={{position: 'fixed', top: 8, right: 8}}>
<button ref={targetRef} {...triggerProps} onClick={() => state.toggle()}>

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 file is pretty old, it looks like we never updated it after some changes because the new stories all render an error overlay due to onPress/etc coming through triggerProps. Easiest thing to do would be to make button a component using useButton and forward ref

Something like:

const Button = React.forwardRef(
  (props: {children: React.ReactNode}, ref: React.RefObject<HTMLButtonElement>): JSX.Element => {
    const {buttonProps} = useButton({...props, ref});
    return (
      <button ref={ref} {...buttonProps}>
        {props.children}
      </button>
    );
  }
);


res.rerender(<AutoWidthExample placement="bottom end" triggerLeft={180} />);

// 180 + 100 - 60 = 220. The stale right inset from the previous placement must

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.

The stale right inset

Was that 320? or what is this referring to?

overlay.style.maxHeight = (window.visualViewport?.height ?? window.innerHeight) + 'px';
}

// Reset the horizontal insets before measuring as well. An auto-width overlay is otherwise

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.

Are there any cases where we don't want to do the style changes? It looks like we only do it sometimes for top/bottom.
What if a width is already specified for the overlay, then we shouldn't need to do this?
Or what if the position is already left?

In general this seems more correct compared to the previous attempt.

@snowystinger

Copy link
Copy Markdown
Member

Looks like yarn lint is failing

One of our chromatic stories is acting weirdly as a result of this. I cannot horizontally scroll this page anymore and it doesn't close the tooltip.
yarn start:chromatic
path=/story/tooltiptriggerrtl--placement-no-flip
compared to the state on main where it used to close the tooltip and scroll. I don't quite know how to classify it yet without knowing more about why it's happening.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spectrum 2 ActionMenu renders too narrow for descriptions on page right Tooltip causing ResizeObserver issue

2 participants