fix(react-aria): Updating scrollIntoView to support ancestral scroll parents + scrollMargin computed styles

change/@fluentui-react-aria-8cb50a9f-693a-4e10-ac94-85a6190f7c7c.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Updating ActiveDescendent scrollIntoView logic to work with ancester scroll containers and scroll margin styles",
+  "packageName": "@fluentui/react-aria",
+  "email": "stevenco@microsoft.com",
+  "dependentChangeType": "patch"
+}

packages/react-components/react-aria/src/activedescendant/scrollIntoView.test.ts

@@ -0,0 +1,190 @@
+import { scrollIntoView } from './scrollIntoView';
+
+describe('scrollIntoView', () => {
+  const mockListboxScrollTo = jest.fn();
+  const mockAncestorScrollTo = jest.fn();
+  let listboxGrandParent: Partial<HTMLElement> = {};
+  let listboxParent: Partial<HTMLElement> = {};
+  let listbox: Partial<HTMLElement> = {};
+
+  beforeEach(() => {
+    listboxGrandParent = {
+      offsetTop: 0,
+      offsetHeight: 50,
+      scrollHeight: 170,
+      scrollTop: 0,
+      offsetParent: null,
+      scrollTo: mockAncestorScrollTo,
+      contains: jest.fn().mockReturnValue(false),
+    };
+    listboxParent = {
+      offsetTop: 20,
+      offsetHeight: 150,
+      scrollHeight: 150,
+      offsetParent: listboxGrandParent as Element,
+      parentElement: listboxGrandParent as HTMLElement,
+      contains: jest.fn().mockReturnValue(false),
+    };
+    listbox = {
+      offsetTop: 40,
+      scrollHeight: 100,
+      scrollTop: 0,
+      offsetHeight: 50,
+      offsetParent: listboxGrandParent as Element,
+      parentElement: listboxParent as HTMLElement,
+      scrollTo: mockListboxScrollTo,
+      contains: jest.fn().mockReturnValue(false),
+    };
+    mockListboxScrollTo.mockClear();
+    mockAncestorScrollTo.mockClear();
+    jest
+      .spyOn(window, 'getComputedStyle')
+      .mockReturnValue({ scrollMarginTop: '0', scrollMarginBottom: '0' } as CSSStyleDeclaration);
+  });
+
+  it('should scroll element above view into view - no scroll margin - scrollable listbox', () => {
+    listbox = {
+      ...listbox,
+      scrollHeight: 100,
+      scrollTop: 100,
+    };
+
+    const option: Partial<HTMLElement> = {
+      offsetHeight: 10,
+      offsetTop: 0,
+      offsetParent: listbox as HTMLElement,
+      parentElement: listbox as HTMLElement,
+      contains: jest.fn().mockReturnValue(false),
+    };
+
+    scrollIntoView(option as HTMLElement);
+
+    expect(mockListboxScrollTo).toHaveBeenCalledWith(0, -2);
+  });
+
+  it('should scroll element above view into view - with scroll margin - scrollable listbox', () => {
+    listbox = {
+      ...listbox,
+      scrollHeight: 100,
+      scrollTop: 100,
+    };
+
+    const option: Partial<HTMLElement> = {
+      offsetHeight: 10,
+      offsetTop: 0,
+      offsetParent: listbox as HTMLElement,
+      parentElement: listbox as HTMLElement,
+      contains: jest.fn().mockReturnValue(false),
+    };
+
+    jest
+      .spyOn(window, 'getComputedStyle')
+      .mockReturnValue({ scrollMarginTop: '10', scrollMarginBottom: '0' } as CSSStyleDeclaration);
+    scrollIntoView(option as HTMLElement);
+
+    expect(mockListboxScrollTo).toHaveBeenCalledWith(0, -12);
+  });
+
+  it('should scroll element below view into view - no scroll margin - scrollable listbox', () => {
+    listbox = {
+      ...listbox,
+      scrollHeight: 100,
+      scrollTop: 0,
+    };
+
+    const option: Partial<HTMLElement> = {
+      offsetHeight: 10,
+      offsetTop: 90,
+      offsetParent: listbox as HTMLElement,
+      parentElement: listbox as HTMLElement,
+      contains: jest.fn().mockReturnValue(false),
+    };
+
+    scrollIntoView(option as HTMLElement);
+
+    expect(mockListboxScrollTo).toHaveBeenCalledWith(0, 52);
+  });
+
+  it('should scroll element below view into view - with scroll margin - scrollable listbox', () => {
+    listbox = {
+      ...listbox,
+      scrollHeight: 100,
+      scrollTop: 0,
+    };
+
+    const option: Partial<HTMLElement> = {
+      offsetHeight: 10,
+      offsetTop: 90,
+      offsetParent: listbox as HTMLElement,
+      parentElement: listbox as HTMLElement,
+      contains: jest.fn().mockReturnValue(false),
+    };
+
+    jest
+      .spyOn(window, 'getComputedStyle')
+      .mockReturnValue({ scrollMarginTop: '0', scrollMarginBottom: '10' } as CSSStyleDeclaration);
+    scrollIntoView(option as HTMLElement);
+
+    expect(mockListboxScrollTo).toHaveBeenCalledWith(0, 62);
+  });
+
+  it('should scroll element above view into view - scrollable ancestor', () => {
+    listboxGrandParent = {
+      ...listboxGrandParent,
+      scrollTop: 170,
+    };
+    listboxParent = {
+      ...listboxParent,
+      offsetParent: listboxGrandParent as Element,
+      parentElement: listboxGrandParent as HTMLElement,
+    };
+    listbox = {
+      ...listbox,
+      scrollHeight: 100,
+      offsetHeight: 100,
+      offsetParent: listboxGrandParent as Element,
+      parentElement: listboxParent as HTMLElement,
+    };
+    const option: Partial<HTMLElement> = {
+      offsetHeight: 10,
+      offsetTop: 0,
+      offsetParent: listboxGrandParent as HTMLElement,
+      parentElement: listbox as HTMLElement,
+      contains: jest.fn().mockReturnValue(false),
+    };
+
+    scrollIntoView(option as HTMLElement);
+
+    expect(mockAncestorScrollTo).toHaveBeenCalledWith(0, -2);
+  });
+
+  it('should scroll element below view into view - scrollable ancestor', () => {
+    listboxGrandParent = {
+      ...listboxGrandParent,
+      scrollTop: 0,
+    };
+    listboxParent = {
+      ...listboxParent,
+      offsetParent: listboxGrandParent as Element,
+      parentElement: listboxGrandParent as HTMLElement,
+    };
+    listbox = {
+      ...listbox,
+      scrollHeight: 100,
+      offsetHeight: 100,
+      offsetParent: listboxGrandParent as Element,
+      parentElement: listboxParent as HTMLElement,
+    };
+    const option: Partial<HTMLElement> = {
+      offsetHeight: 10,
+      offsetTop: 160,
+      offsetParent: listboxGrandParent as HTMLElement,
+      parentElement: listbox as HTMLElement,
+      contains: jest.fn().mockReturnValue(false),
+    };
+
+    scrollIntoView(option as HTMLElement);
+
+    expect(mockAncestorScrollTo).toHaveBeenCalledWith(0, 122);
+  });
+});

packages/react-components/react-aria/src/activedescendant/scrollIntoView.ts

@@ -1,28 +1,71 @@
-export const scrollIntoView = (
-  target: HTMLElement | null | undefined,
-  scrollParent: HTMLElement | null | undefined,
-) => {
-  if (!target || !scrollParent) {
+export const scrollIntoView = (target: HTMLElement | null | undefined) => {
+  if (!target) {
     return;
   }
 
-  if (scrollParent.offsetHeight >= scrollParent.scrollHeight) {
+  const scrollParent = findScrollableParent(target.parentElement as HTMLElement);
+  if (!scrollParent) {
     return;
   }
 
-  const { offsetHeight, offsetTop } = target;
+  const { offsetHeight } = target;
+  const offsetTop = getTotalOffsetTop(target, scrollParent);
+
+  const { scrollMarginTop, scrollMarginBottom } = getScrollMargins(target);
+
   const { offsetHeight: parentOffsetHeight, scrollTop } = scrollParent;
 
-  const isAbove = offsetTop < scrollTop;
-  const isBelow = offsetTop + offsetHeight > scrollTop + parentOffsetHeight;
+  const isAbove = offsetTop - scrollMarginTop < scrollTop;
+  const isBelow = offsetTop + offsetHeight + scrollMarginBottom > scrollTop + parentOffsetHeight;
 
   const buffer = 2;
 
   if (isAbove) {
-    scrollParent.scrollTo(0, offsetTop - buffer);
+    scrollParent.scrollTo(0, offsetTop - scrollMarginTop - buffer);
+  } else if (isBelow) {
+    scrollParent.scrollTo(0, offsetTop + offsetHeight + scrollMarginBottom - parentOffsetHeight + buffer);
+  }
+};
+
+const findScrollableParent = (element: HTMLElement | null): HTMLElement | null => {
+  if (!element) {
+    return null;
+  }
+
+  if (element.scrollHeight > element.offsetHeight) {
+    return element;
+  }
+
+  return findScrollableParent(element.parentElement);
+};
+
+const getTotalOffsetTop = (element: HTMLElement, scrollParent: HTMLElement): number => {
+  if (!element || element === scrollParent) {
+    return 0;
   }
 
-  if (isBelow) {
-    scrollParent.scrollTo(0, offsetTop - parentOffsetHeight + offsetHeight + buffer);
+  if (element.contains(scrollParent)) {
+    // subtract the scroll parent's offset top from the running total if the offsetParent is above it
+    return scrollParent.offsetTop * -1;
   }
+
+  return element.offsetTop + getTotalOffsetTop(element.offsetParent as HTMLElement, scrollParent);
+};
+
+const getScrollMargins = (element: HTMLElement) => {
+  const computedStyles = getComputedStyle(element);
+  const scrollMarginTop =
+    getIntValueOfComputedStyle(computedStyles.scrollMarginTop) ??
+    getIntValueOfComputedStyle(computedStyles.scrollMarginBlockStart);
+  const scrollMarginBottom =
+    getIntValueOfComputedStyle(computedStyles.scrollMarginBottom) ??
+    getIntValueOfComputedStyle(computedStyles.scrollMarginBlockEnd);
+  return {
+    scrollMarginTop,
+    scrollMarginBottom,
+  };
+};
+
+const getIntValueOfComputedStyle = (computedStyle: string) => {
+  return computedStyle ? parseInt(computedStyle, 10) : 0;
 };

packages/react-components/react-aria/src/activedescendant/useActiveDescendant.ts

@@ -67,15 +67,15 @@ export function useActiveDescendant<TActiveParentElement extends HTMLElement, TL
 
       blurActiveDescendant();
 
-      scrollIntoView(nextActive, listboxRef.current);
+      scrollIntoView(nextActive);
       setAttribute(nextActive.id);
       nextActive.setAttribute(ACTIVEDESCENDANT_ATTRIBUTE, '');
 
       if (focusVisibleRef.current) {
         nextActive.setAttribute(ACTIVEDESCENDANT_FOCUSVISIBLE_ATTRIBUTE, '');
       }
     },
-    [listboxRef, blurActiveDescendant, setAttribute],
+    [blurActiveDescendant, setAttribute],
   );
 
   const controller: ActiveDescendantImperativeRef = React.useMemo(