fix: Overlay positioning incorrect when animation starts on first render (#8789)

* fix: Overlay positioning incorrect when animation starts on first render

* fix tests

* Revert collection incomplete tracking

Doesn't work in React 17

* Use offsetWidth and offsetHeight instead of getBoundingClientRect

Author: devongovett

packages/@react-aria/collections/src/BaseCollection.ts

@@ -140,7 +140,6 @@ export class BaseCollection<T> implements ICollection<Node<T>> {
   private lastKey: Key | null = null;
   private frozen = false;
   private itemCount: number = 0;
-  isComplete = true;
 
   get size(): number {
     return this.itemCount;

packages/@react-aria/collections/src/CollectionBuilder.tsx

@@ -116,7 +116,6 @@ function useCollectionDocument<T extends object, C extends BaseCollection<T>>(cr
   let collection = useSyncExternalStore(subscribe, getSnapshot, getServerSnapshot);
   useLayoutEffect(() => {
     document.isMounted = true;
-    document.isInitialRender = false;
     return () => {
       // Mark unmounted so we can skip all of the collection updates caused by
       // React calling removeChild on every item in the collection.

packages/@react-aria/collections/src/Document.ts

@@ -416,7 +416,6 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
   nodeId = 0;
   nodesByProps: WeakMap<object, ElementNode<T>> = new WeakMap<object, ElementNode<T>>();
   isMounted = true;
-  isInitialRender = true;
   private collection: C;
   private nextCollection: C | null = null;
   private subscriptions: Set<() => void> = new Set();
@@ -523,10 +522,6 @@ export class Document<T, C extends BaseCollection<T> = BaseCollection<T>> extend
         this.nextCollection = null;
       }
     }
-
-    if (this.isInitialRender) {
-      this.collection.isComplete = false;
-    }
   }
 
   queueUpdate(): void {

packages/@react-aria/overlays/src/calculatePosition.ts

@@ -127,7 +127,7 @@ function getContainerDimensions(containerNode: Element): Dimensions {
       left = visualViewport.offsetLeft;
     }
   } else {
-    ({width, height, top, left} = getOffset(containerNode));
+    ({width, height, top, left} = getOffset(containerNode, false));
     scroll.top = containerNode.scrollTop;
     scroll.left = containerNode.scrollLeft;
     totalWidth = width;
@@ -488,15 +488,15 @@ export function calculatePosition(opts: PositionOpts): PositionResult {
   let isViewportContainer = container === document.documentElement;
   const containerPositionStyle = window.getComputedStyle(container).position;
   let isContainerPositioned = !!containerPositionStyle && containerPositionStyle !== 'static';
-  let childOffset: Offset = isViewportContainer ? getOffset(targetNode) : getPosition(targetNode, container);
+  let childOffset: Offset = isViewportContainer ? getOffset(targetNode, false) : getPosition(targetNode, container, false);
 
   if (!isViewportContainer) {
     let {marginTop, marginLeft} = window.getComputedStyle(targetNode);
     childOffset.top += parseInt(marginTop, 10) || 0;
     childOffset.left += parseInt(marginLeft, 10) || 0;
   }
 
-  let overlaySize: Offset = getOffset(overlayNode);
+  let overlaySize: Offset = getOffset(overlayNode, true);
   let margins = getMargins(overlayNode);
   overlaySize.width += (margins.left ?? 0) + (margins.right ?? 0);
   overlaySize.height += (margins.top ?? 0) + (margins.bottom ?? 0);
@@ -507,7 +507,7 @@ export function calculatePosition(opts: PositionOpts): PositionResult {
   // If the container is the HTML element wrapping the body element, the retrieved scrollTop/scrollLeft will be equal to the
   // body element's scroll. Set the container's scroll values to 0 since the overlay's edge position value in getDelta don't then need to be further offset
   // by the container scroll since they are essentially the same containing element and thus in the same coordinate system
-  let containerOffsetWithBoundary: Offset = boundaryElement.tagName === 'BODY' ? getOffset(container) : getPosition(container, boundaryElement);
+  let containerOffsetWithBoundary: Offset = boundaryElement.tagName === 'BODY' ? getOffset(container, false) : getPosition(container, boundaryElement, false);
   if (container.tagName === 'HTML' && boundaryElement.tagName === 'BODY') {
     containerDimensions.scroll.top = 0;
     containerDimensions.scroll.left = 0;
@@ -533,8 +533,21 @@ export function calculatePosition(opts: PositionOpts): PositionResult {
   );
 }
 
-function getOffset(node: Element): Offset {
+export function getRect(node: Element, ignoreScale: boolean) {
   let {top, left, width, height} = node.getBoundingClientRect();
+
+  // Use offsetWidth and offsetHeight if this is an HTML element, so that 
+  // the size is not affected by scale transforms.
+  if (ignoreScale && node instanceof node.ownerDocument.defaultView!.HTMLElement) {
+    width = node.offsetWidth;
+    height = node.offsetHeight;
+  }
+
+  return {top, left, width, height};
+}
+
+function getOffset(node: Element, ignoreScale: boolean): Offset {
+  let {top, left, width, height} = getRect(node, ignoreScale);
   let {scrollTop, scrollLeft, clientTop, clientLeft} = document.documentElement;
   return {
     top: top + scrollTop - clientTop,
@@ -544,15 +557,14 @@ function getOffset(node: Element): Offset {
   };
 }
 
-function getPosition(node: Element, parent: Element): Offset {
+function getPosition(node: Element, parent: Element, ignoreScale: boolean): Offset {
   let style = window.getComputedStyle(node);
   let offset: Offset;
   if (style.position === 'fixed') {
-    let {top, left, width, height} = node.getBoundingClientRect();
-    offset = {top, left, width, height};
+    offset = getRect(node, ignoreScale);
   } else {
-    offset = getOffset(node);
-    let parentOffset = getOffset(parent);
+    offset = getOffset(node, ignoreScale);
+    let parentOffset = getOffset(parent, ignoreScale);
     let parentStyle = window.getComputedStyle(parent);
     parentOffset.top += (parseInt(parentStyle.borderTopWidth, 10) || 0) - parent.scrollTop;
     parentOffset.left += (parseInt(parentStyle.borderLeftWidth, 10) || 0) - parent.scrollLeft;

packages/@react-aria/overlays/src/useOverlayPosition.ts

@@ -10,7 +10,7 @@
  * governing permissions and limitations under the License.
  */
 
-import {calculatePosition, PositionResult} from './calculatePosition';
+import {calculatePosition, getRect, PositionResult} from './calculatePosition';
 import {DOMAttributes, RefObject} from '@react-types/shared';
 import {Placement, PlacementAxis, PositionProps} from '@react-types/overlays';
 import {useCallback, useEffect, useRef, useState} from 'react';
@@ -149,17 +149,6 @@ export function useOverlayPosition(props: AriaPositionProps): PositionAria {
       return;
     }
 
-    // Delay updating the position until children are finished rendering (e.g. collections).
-    if (overlayRef.current.querySelector('[data-react-aria-incomplete]')) {
-      return;
-    }
-
-    // Don't update while the overlay is animating.
-    // Things like scale animations can mess up positioning by affecting the overlay's computed size.
-    if (overlayRef.current.getAnimations?.().length > 0) {
-      return;
-    }
-
     // Determine a scroll anchor based on the focused element.
     // This stores the offset of the anchor element from the scroll container
     // so it can be restored after repositioning. This way if the overlay height
@@ -200,7 +189,7 @@ export function useOverlayPosition(props: AriaPositionProps): PositionAria {
       offset,
       crossOffset,
       maxHeight,
-      arrowSize: arrowSize ?? arrowRef?.current?.getBoundingClientRect().width ?? 0,
+      arrowSize: arrowSize ?? (arrowRef?.current ? getRect(arrowRef.current, true).width : 0),
       arrowBoundaryOffset
     });
 

packages/@react-aria/overlays/test/calculatePosition.test.ts

@@ -66,6 +66,9 @@ function createElementWithDimensions(elemName, dimensions, margins = {}) {
     bottom: dimensions.bottom || 0
   });
 
+  jest.spyOn(elem, 'offsetWidth', 'get').mockImplementation(() => dimensions.width || 0);
+  jest.spyOn(elem, 'offsetHeight', 'get').mockImplementation(() => dimensions.height || 0);
+
   jest.spyOn(elem, 'scrollWidth', 'get').mockImplementation(() => dimensions.width || 0);
   jest.spyOn(elem, 'scrollHeight', 'get').mockImplementation(() => dimensions.height || 0);
 

packages/@react-aria/overlays/test/useOverlayPosition.test.tsx

@@ -51,6 +51,13 @@ describe('useOverlayPosition', function () {
   beforeEach(() => {
     Object.defineProperty(HTMLElement.prototype, 'clientHeight', {configurable: true, value: 768});
     Object.defineProperty(HTMLElement.prototype, 'clientWidth', {configurable: true, value: 500});
+
+    jest.spyOn(HTMLElement.prototype, 'offsetWidth', 'get').mockImplementation(function (this: HTMLElement) {
+      return parseInt(this.style.width, 10) || 0;
+    });
+    jest.spyOn(HTMLElement.prototype, 'offsetHeight', 'get').mockImplementation(function (this: HTMLElement) {
+      return parseInt(this.style.height, 10) || 0;
+    });
   });
 
   it('should position the overlay relative to the trigger', function () {

packages/react-aria-components/src/Menu.tsx

@@ -241,7 +241,6 @@ function MenuInner<T extends object>({props, collection, menuRef: ref}: MenuInne
         ref={ref as RefObject<HTMLDivElement>}
         slot={props.slot || undefined}
         data-empty={state.collection.size === 0 || undefined}
-        data-react-aria-incomplete={!(state.collection as BaseCollection<T>).isComplete || undefined}
         onScroll={props.onScroll}>
         <Provider
           values={[

packages/react-aria-components/stories/Popover.stories.tsx

@@ -30,6 +30,10 @@ export default {
         'left', 'left top', 'left bottom', 'start', 'start top', 'start bottom',
         'right', 'right top', 'right bottom', 'end', 'end top', 'end bottom'
       ]
+    },
+    animation: {
+      control: 'radio',
+      options: ['transition', 'animation', 'animation-delayed']
     }
   }
 } as Meta<typeof Popover>;
@@ -41,7 +45,7 @@ export const PopoverExample: PopoverStory = (args) => (
     <Button>Open popover</Button>
     <Popover
       {...args}
-      className={styles.popover}
+      className={`${styles['popover-base']} ${styles[(args as any).animation]}`}
       style={{
         background: 'Canvas',
         color: 'CanvasText',

packages/react-aria-components/stories/Tooltip.stories.tsx

@@ -30,6 +30,10 @@ export default {
         'left', 'left top', 'left bottom', 'start', 'start top', 'start bottom',
         'right', 'right top', 'right bottom', 'end', 'end top', 'end bottom'
       ]
+    },
+    animation: {
+      control: 'radio',
+      options: ['transition', 'animation', 'animation-delayed']
     }
   }
 } as Meta<typeof Tooltip>;
@@ -42,7 +46,7 @@ export const TooltipExample: TooltipStory = (args) => (
     <Button>Tooltip trigger</Button>
     <Tooltip
       {...args}
-      className={styles.tooltip}
+      className={`${styles['tooltip-base']} ${styles[(args as any).animation]}`}
       offset={5}
       style={{
         background: 'Canvas',