fix: Skip popover position updates if it happened offscreen (#3568)

Author: just-boris

src/annotation-context/__integ__/annotation-scroll.test.ts

@@ -8,11 +8,17 @@ import createWrapper from '../../../lib/components/test-utils/selectors';
 const wrapper = createWrapper();
 const annotationWrapper = wrapper.findAnnotation();
 
+function setupTest(testFn: (page: BasePageObject) => Promise<void>) {
+  return useBrowser(async browser => {
+    const page = new BasePageObject(browser);
+    await browser.url('#/light/annotation-context/annotation-scroll');
+    await testFn(page);
+  });
+}
+
 test(
   'scrolls annotation in view with fixed elements',
-  useBrowser(async browser => {
-    await browser.url('#/light/annotation-context/annotation-scroll');
-    const page = new BasePageObject(browser);
+  setupTest(async page => {
     const nextButtonSelector = annotationWrapper.findNextButton().toSelector();
     await page.waitForVisible(nextButtonSelector);
     await page.click(nextButtonSelector);
@@ -37,6 +43,21 @@ test(
   })
 );
 
+test(
+  'does not update popover position when annotation is rendered offscreen',
+  setupTest(async page => {
+    await expect(page.isDisplayedInViewport(annotationWrapper.toSelector())).resolves.toEqual(true);
+    // scroll to the page bottom to make annotation offscreen
+    await page.windowScrollTo({ top: 1200 });
+    await expect(page.isDisplayedInViewport(annotationWrapper.toSelector())).resolves.toEqual(false);
+    const positionBefore = await page.getBoundingBox(annotationWrapper.toSelector());
+    // click somewhere to trigger the regression
+    await page.click('main');
+    const positionAfter = await page.getBoundingBox(annotationWrapper.toSelector());
+    expect(positionBefore).toEqual(positionAfter);
+  })
+);
+
 function overlap(one: ElementRect, two: ElementRect) {
   if (one.right < two.left || one.left > two.right) {
     return false;

src/popover/container.tsx

@@ -4,7 +4,7 @@ import React, { useLayoutEffect, useRef } from 'react';
 import clsx from 'clsx';
 
 import { nodeContains } from '@cloudscape-design/component-toolkit/dom';
-import { useResizeObserver } from '@cloudscape-design/component-toolkit/internal';
+import { getLogicalBoundingClientRect, useResizeObserver } from '@cloudscape-design/component-toolkit/internal';
 
 import { useVisualRefresh } from '../internal/hooks/use-visual-mode';
 import { InternalPosition, PopoverProps } from './interfaces';
@@ -18,7 +18,7 @@ interface PopoverContainerProps {
   getTrack?: () => null | HTMLElement | SVGElement;
   /**
     Used to update the container position in case track or track position changes:
-    
+
     const trackRef = useRef<Element>(null)
     return (<>
       <Track style={getPosition(selectedItemId)} ref={trackRef} />
@@ -131,6 +131,13 @@ export default function PopoverContainer({
         return;
       }
 
+      // Do not update position if popover moved offscreen
+      const popoverOffset = popoverRef.current && getLogicalBoundingClientRect(popoverRef.current);
+      // istanbul ignore if - tested via integration tests
+      if (!popoverOffset || popoverOffset.insetBlockStart < 0 || popoverOffset.insetBlockEnd > window.innerHeight) {
+        return;
+      }
+
       // Continuously update the popover position for one second to account for any layout changes
       // and animations. On browsers where `requestIdleCallback` is supported,
       // this runs only while the CPU is otherwise idle. In other browsers (mainly Safari), we call it