fix: scrollIntoView should respect scroll-margin (#8715)

* fix: scrollIntoView should respect scroll-margin

* chore: formatting

* feat: add scroll-margin story

* feat: add smooth scroll story

---------

Co-authored-by: Robert Snow <rsnow@adobe.com>

Author: nwidynski

packages/@react-aria/utils/src/scrollIntoView.ts

@@ -11,6 +11,7 @@
  */
 
 import {getScrollParents} from './getScrollParents';
+import {isChrome} from './platform';
 
 interface ScrollIntoViewportOpts {
   /** The optional containing element of the target to be centered in the viewport. */
@@ -40,32 +41,64 @@ export function scrollIntoView(scrollView: HTMLElement, element: HTMLElement): v
     scrollPaddingLeft
   } = getComputedStyle(scrollView);
 
+  // Account for scroll margin of the element
+  let {
+    scrollMarginTop,
+    scrollMarginRight,
+    scrollMarginBottom,
+    scrollMarginLeft
+  } = getComputedStyle(element);
+
   let borderAdjustedX = x + parseInt(borderLeftWidth, 10);
   let borderAdjustedY = y + parseInt(borderTopWidth, 10);
   // Ignore end/bottom border via clientHeight/Width instead of offsetHeight/Width
   let maxX = borderAdjustedX + scrollView.clientWidth;
   let maxY = borderAdjustedY + scrollView.clientHeight;
 
-  // Get scroll padding values as pixels - defaults to 0 if no scroll padding
+  // Get scroll padding / margin values as pixels - defaults to 0 if no scroll padding / margin
   // is used.
   let scrollPaddingTopNumber = parseInt(scrollPaddingTop, 10) || 0;
   let scrollPaddingBottomNumber = parseInt(scrollPaddingBottom, 10) || 0;
   let scrollPaddingRightNumber = parseInt(scrollPaddingRight, 10) || 0;
   let scrollPaddingLeftNumber = parseInt(scrollPaddingLeft, 10) || 0;
+  let scrollMarginTopNumber = parseInt(scrollMarginTop, 10) || 0;
+  let scrollMarginBottomNumber = parseInt(scrollMarginBottom, 10) || 0;
+  let scrollMarginRightNumber = parseInt(scrollMarginRight, 10) || 0;
+  let scrollMarginLeftNumber = parseInt(scrollMarginLeft, 10) || 0;
+
+  let targetLeft = offsetX - scrollMarginLeftNumber;
+  let targetRight = offsetX + width + scrollMarginRightNumber;
+  let targetTop = offsetY - scrollMarginTopNumber;
+  let targetBottom = offsetY + height + scrollMarginBottomNumber;
 
-  if (offsetX <= x + scrollPaddingLeftNumber) {
-    x = offsetX - parseInt(borderLeftWidth, 10) - scrollPaddingLeftNumber;
-  } else if (offsetX + width > maxX - scrollPaddingRightNumber) {
-    x += offsetX + width - maxX + scrollPaddingRightNumber;
+  let scrollPortLeft = x + parseInt(borderLeftWidth, 10) + scrollPaddingLeftNumber;
+  let scrollPortRight = maxX - scrollPaddingRightNumber;
+  let scrollPortTop = y + parseInt(borderTopWidth, 10) + scrollPaddingTopNumber;
+  let scrollPortBottom = maxY - scrollPaddingBottomNumber;
+
+  if (targetLeft > scrollPortLeft || targetRight < scrollPortRight) {
+    if (targetLeft <= x + scrollPaddingLeftNumber) {
+      x = targetLeft - parseInt(borderLeftWidth, 10) - scrollPaddingLeftNumber;
+    } else if (targetRight > maxX - scrollPaddingRightNumber) {
+      x += targetRight - maxX + scrollPaddingRightNumber;
+    }
   }
-  if (offsetY <= borderAdjustedY + scrollPaddingTopNumber) {
-    y = offsetY - parseInt(borderTopWidth, 10) - scrollPaddingTopNumber;
-  } else if (offsetY + height > maxY - scrollPaddingBottomNumber) {
-    y += offsetY + height - maxY + scrollPaddingBottomNumber;
+
+  if (targetTop > scrollPortTop || targetBottom < scrollPortBottom) {
+    if (targetTop <= borderAdjustedY + scrollPaddingTopNumber) {
+      y = targetTop - parseInt(borderTopWidth, 10) - scrollPaddingTopNumber;
+    } else if (targetBottom > maxY - scrollPaddingBottomNumber) {
+      y += targetBottom - maxY + scrollPaddingBottomNumber;
+    }
+  }
+
+  if (process.env.NODE_ENV === 'test') {
+    scrollView.scrollLeft = x;
+    scrollView.scrollTop = y;
+    return;
   }
 
-  scrollView.scrollLeft = x;
-  scrollView.scrollTop = y;
+  scrollView.scrollTo({left: x, top: y});
 }
 
 /**
@@ -101,8 +134,9 @@ export function scrollIntoViewport(targetElement: Element | null, opts?: ScrollI
   if (targetElement && document.contains(targetElement)) {
     let root = document.scrollingElement || document.documentElement;
     let isScrollPrevented = window.getComputedStyle(root).overflow === 'hidden';
-    // If scrolling is not currently prevented then we aren’t in a overlay nor is a overlay open, just use element.scrollIntoView to bring the element into view
-    if (!isScrollPrevented) {
+    // If scrolling is not currently prevented then we aren't in a overlay nor is a overlay open, just use element.scrollIntoView to bring the element into view
+    // Also ignore in chrome because of this bug: https://issues.chromium.org/issues/40074749
+    if (!isScrollPrevented && !isChrome()) {
       let {left: originalLeft, top: originalTop} = targetElement.getBoundingClientRect();
 
       // use scrollIntoView({block: 'nearest'}) instead of .focus to check if the element is fully in view or not since .focus()

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

@@ -743,6 +743,46 @@ export const AsyncListBoxVirtualized: StoryFn<typeof AsyncListBoxRender> = (args
   );
 };
 
+export const ListBoxScrollMargin: ListBoxStory = (args) => {
+  let items: {id: number, name: string, description: string}[] = [];
+  for (let i = 0; i < 100; i++) {
+    items.push({id: i, name: `Item ${i}`, description: `Description ${i}`});
+  }
+  return (
+    <ListBox 
+      className={styles.menu} 
+      {...args}
+      aria-label="test listbox" 
+      style={{height: 200, width: 100, overflow: 'scroll'}} 
+      items={items}>
+      {item => (
+        <MyListBoxItem style={{scrollMargin: 10, width: 150, display: 'flex', padding: '2px 20px', justifyContent: 'space-between'}}>
+          <span>{item.name}</span>
+          <span>{item.description}</span>
+        </MyListBoxItem>
+      )}
+    </ListBox>
+  );
+};
+
+export const ListBoxSmoothScroll: ListBoxStory = (args) => {
+  let items: {id: number, name: string}[] = [];
+  for (let i = 0; i < 100; i++) {
+    items.push({id: i, name: `Item ${i}`});
+  }
+  return (
+    <ListBox 
+      className={styles.menu} 
+      {...args} 
+      aria-label="test listbox" 
+      style={{height: 200, width: 200, overflow: 'scroll', display: 'grid', gridTemplateColumns: 'repeat(4, 80px)', scrollBehavior: 'smooth'}} 
+      items={items} 
+      layout="grid">
+      {item => <MyListBoxItem style={{minHeight: 32}}>{item.name}</MyListBoxItem>}
+    </ListBox>
+  );
+};
+
 AsyncListBoxVirtualized.story = {
   args: {
     delay: 50