From 0fec7011641d60aa622522f04f67b82df15de227 Mon Sep 17 00:00:00 2001 From: Sean Monahan Date: Thu, 23 May 2024 12:51:28 -0700 Subject: [PATCH 1/4] chore: revert globals changes --- .../src/activedescendant/scrollIntoView.ts | 17 +++++++++++------ .../src/activedescendant/useActiveDescendant.ts | 7 ++----- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/packages/react-components/react-aria/src/activedescendant/scrollIntoView.ts b/packages/react-components/react-aria/src/activedescendant/scrollIntoView.ts index 3ca94fe44b16e..a255586f80f71 100644 --- a/packages/react-components/react-aria/src/activedescendant/scrollIntoView.ts +++ b/packages/react-components/react-aria/src/activedescendant/scrollIntoView.ts @@ -1,11 +1,8 @@ -export const scrollIntoView = (target: HTMLElement | null | undefined, winArg?: Window | null) => { +export const scrollIntoView = (target: HTMLElement | null | undefined) => { if (!target) { return; } - // eslint-disable-next-line no-restricted-globals - const win = winArg ?? window; - const scrollParent = findScrollableParent(target.parentElement as HTMLElement); if (!scrollParent) { return; @@ -14,7 +11,7 @@ export const scrollIntoView = (target: HTMLElement | null | undefined, winArg?: const { offsetHeight } = target; const offsetTop = getTotalOffsetTop(target, scrollParent); - const { scrollMarginTop, scrollMarginBottom } = getScrollMargins(target, win); + const { scrollMarginTop, scrollMarginBottom } = getScrollMargins(target); const { offsetHeight: parentOffsetHeight, scrollTop } = scrollParent; @@ -55,7 +52,15 @@ const getTotalOffsetTop = (element: HTMLElement, scrollParent: HTMLElement): num return element.offsetTop + getTotalOffsetTop(element.offsetParent as HTMLElement, scrollParent); }; -const getScrollMargins = (element: HTMLElement, win: Window) => { +const getScrollMargins = (element: HTMLElement) => { + const win = element.ownerDocument.defaultView; + if (!win) { + return { + scrollMarginTop: 0, + scrollMarginBottom: 0, + }; + } + const computedStyles = win.getComputedStyle(element); const scrollMarginTop = getIntValueOfComputedStyle(computedStyles.scrollMarginTop) ?? diff --git a/packages/react-components/react-aria/src/activedescendant/useActiveDescendant.ts b/packages/react-components/react-aria/src/activedescendant/useActiveDescendant.ts index 33413f848f42a..0512298e8e968 100644 --- a/packages/react-components/react-aria/src/activedescendant/useActiveDescendant.ts +++ b/packages/react-components/react-aria/src/activedescendant/useActiveDescendant.ts @@ -1,5 +1,4 @@ import * as React from 'react'; -import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; import { useEventCallback, useMergedRefs } from '@fluentui/react-utilities'; import { useOnKeyboardNavigationChange } from '@fluentui/react-tabster'; import { useOptionWalker } from './useOptionWalker'; @@ -33,8 +32,6 @@ export function useActiveDescendant(null); const activeParentRef = React.useRef(null); const attributeVisibilityRef = React.useRef(true); - const { targetDocument } = useFluent(); - const win = targetDocument?.defaultView; const removeAttribute = React.useCallback(() => { activeParentRef.current?.removeAttribute('aria-activedescendant'); @@ -90,7 +87,7 @@ export function useActiveDescendant Date: Thu, 23 May 2024 13:35:07 -0700 Subject: [PATCH 2/4] remove test --- ...-95b07b56-fae4-4b3b-bb62-beab45149e11.json | 7 ++++ .../activedescendant/scrollIntoView.test.ts | 32 ------------------- 2 files changed, 7 insertions(+), 32 deletions(-) create mode 100644 change/@fluentui-react-aria-95b07b56-fae4-4b3b-bb62-beab45149e11.json diff --git a/change/@fluentui-react-aria-95b07b56-fae4-4b3b-bb62-beab45149e11.json b/change/@fluentui-react-aria-95b07b56-fae4-4b3b-bb62-beab45149e11.json new file mode 100644 index 0000000000000..bbe84f8a3882c --- /dev/null +++ b/change/@fluentui-react-aria-95b07b56-fae4-4b3b-bb62-beab45149e11.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "chore: remove optional \"win\" argument", + "packageName": "@fluentui/react-aria", + "email": "seanmonahan@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-aria/src/activedescendant/scrollIntoView.test.ts b/packages/react-components/react-aria/src/activedescendant/scrollIntoView.test.ts index 9188534869416..7d33854079d96 100644 --- a/packages/react-components/react-aria/src/activedescendant/scrollIntoView.test.ts +++ b/packages/react-components/react-aria/src/activedescendant/scrollIntoView.test.ts @@ -187,36 +187,4 @@ describe('scrollIntoView', () => { expect(mockAncestorScrollTo).toHaveBeenCalledWith(0, 122); }); - - it('should use the provided `window` argument', () => { - 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 = { - offsetHeight: 10, - offsetTop: 160, - offsetParent: listboxGrandParent as HTMLElement, - parentElement: listbox as HTMLElement, - contains: jest.fn().mockReturnValue(false), - }; - - const getComputedStyleSpy = jest.spyOn(window, 'getComputedStyle'); - - scrollIntoView(option as HTMLElement); - - expect(getComputedStyleSpy).toHaveBeenCalledTimes(1); - }); }); From 4db3e98a317109d53e2ce145366ebd4f59b81cb2 Mon Sep 17 00:00:00 2001 From: Sean Monahan Date: Thu, 23 May 2024 14:23:38 -0700 Subject: [PATCH 3/4] SSR fix --- .../react-aria/src/activedescendant/scrollIntoView.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-components/react-aria/src/activedescendant/scrollIntoView.ts b/packages/react-components/react-aria/src/activedescendant/scrollIntoView.ts index a255586f80f71..771f4941e3a08 100644 --- a/packages/react-components/react-aria/src/activedescendant/scrollIntoView.ts +++ b/packages/react-components/react-aria/src/activedescendant/scrollIntoView.ts @@ -53,7 +53,7 @@ const getTotalOffsetTop = (element: HTMLElement, scrollParent: HTMLElement): num }; const getScrollMargins = (element: HTMLElement) => { - const win = element.ownerDocument.defaultView; + const win = element.ownerDocument?.defaultView; if (!win) { return { scrollMarginTop: 0, From 0593c35169b0b5aa9c5f60310ebc202b78c66bf5 Mon Sep 17 00:00:00 2001 From: Sean Monahan Date: Thu, 23 May 2024 15:37:33 -0700 Subject: [PATCH 4/4] update test mocking --- .../react-aria/src/activedescendant/scrollIntoView.test.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/react-components/react-aria/src/activedescendant/scrollIntoView.test.ts b/packages/react-components/react-aria/src/activedescendant/scrollIntoView.test.ts index 7d33854079d96..650471f168c68 100644 --- a/packages/react-components/react-aria/src/activedescendant/scrollIntoView.test.ts +++ b/packages/react-components/react-aria/src/activedescendant/scrollIntoView.test.ts @@ -75,6 +75,9 @@ describe('scrollIntoView', () => { offsetParent: listbox as HTMLElement, parentElement: listbox as HTMLElement, contains: jest.fn().mockReturnValue(false), + ownerDocument: { + defaultView: window, + } as Document, }; jest @@ -118,6 +121,9 @@ describe('scrollIntoView', () => { offsetParent: listbox as HTMLElement, parentElement: listbox as HTMLElement, contains: jest.fn().mockReturnValue(false), + ownerDocument: { + defaultView: window, + } as Document, }; jest