From 6f6a793bfbfaf44ab773e5cd9f366d2e30c08b42 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 9 Sep 2021 17:23:42 -0700 Subject: [PATCH 1/4] Restore scroll position when tabbing back into collection --- .../selection/src/useSelectableCollection.ts | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 16dddf08820..d0981a1e3d2 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -13,7 +13,7 @@ import {FocusEvent, HTMLAttributes, Key, KeyboardEvent, RefObject, useEffect, useRef} from 'react'; import {focusSafely, getFocusableTreeWalker} from '@react-aria/focus'; import {FocusStrategy, KeyboardDelegate} from '@react-types/shared'; -import {focusWithoutScrolling, isMac, mergeProps} from '@react-aria/utils'; +import {focusWithoutScrolling, isMac, mergeProps, useEvent} from '@react-aria/utils'; import {MultipleSelectionManager} from '@react-stately/selection'; import {useLocale} from '@react-aria/i18n'; import {useTypeSelect} from './useTypeSelect'; @@ -264,6 +264,15 @@ export function useSelectableCollection(options: SelectableCollectionOptions): S } }; + // Store the scroll position so we can restore it later. + let scrollPos = useRef({top: 0, left: 0}); + useEvent(scrollRef, 'scroll', isVirtualized ? null : () => { + scrollPos.current = { + top: scrollRef.current.scrollTop, + left: scrollRef.current.scrollLeft + }; + }); + let onFocus = (e: FocusEvent) => { if (manager.isFocused) { // If a focus event bubbled through a portal, reset focus state. @@ -291,6 +300,17 @@ export function useSelectableCollection(options: SelectableCollectionOptions): S } else { manager.setFocusedKey(manager.firstSelectedKey ?? delegate.getFirstKey()); } + } else if (!isVirtualized) { + let element = scrollRef.current.querySelector(`[data-key="${manager.focusedKey}"]`) as HTMLElement; + if (element) { + // This prevents a flash of focus on the first/last element in the collection + focusWithoutScrolling(element); + + // Restore the scroll position to what it was before, and then scroll the focused element into view. + scrollRef.current.scrollTop = scrollPos.current.top; + scrollRef.current.scrollLeft = scrollPos.current.left; + scrollIntoView(scrollRef.current, element); + } } }; From dae05859600a502252fd8d9fc8a727f68e4664f7 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 9 Sep 2021 17:33:04 -0700 Subject: [PATCH 2/4] Restore scroll position even if focused key is outside the scrollable area --- .../@react-aria/selection/src/useSelectableCollection.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index d0981a1e3d2..2b9f8a60a91 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -301,14 +301,15 @@ export function useSelectableCollection(options: SelectableCollectionOptions): S manager.setFocusedKey(manager.firstSelectedKey ?? delegate.getFirstKey()); } } else if (!isVirtualized) { + // Restore the scroll position to what it was before. + scrollRef.current.scrollTop = scrollPos.current.top; + scrollRef.current.scrollLeft = scrollPos.current.left; + + // Refocus and scroll the focused item into view if it exists within the scrollable region. let element = scrollRef.current.querySelector(`[data-key="${manager.focusedKey}"]`) as HTMLElement; if (element) { // This prevents a flash of focus on the first/last element in the collection focusWithoutScrolling(element); - - // Restore the scroll position to what it was before, and then scroll the focused element into view. - scrollRef.current.scrollTop = scrollPos.current.top; - scrollRef.current.scrollLeft = scrollPos.current.left; scrollIntoView(scrollRef.current, element); } } From cb935c0a179885b108c0bdd63d85e95150389d93 Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 10 Sep 2021 11:48:42 -0700 Subject: [PATCH 3/4] fixing side nav test --- packages/@react-aria/sidenav/test/useSideNav.test.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/@react-aria/sidenav/test/useSideNav.test.js b/packages/@react-aria/sidenav/test/useSideNav.test.js index 43f3ed81dbe..252c6ca00e9 100644 --- a/packages/@react-aria/sidenav/test/useSideNav.test.js +++ b/packages/@react-aria/sidenav/test/useSideNav.test.js @@ -20,9 +20,15 @@ describe('useSideNav', function () { let mockLayout = new ListLayout({ rowHeight: 40 }); + let mockRef = { + current: { + addEventListener: () => jest.fn(), + removeEventListener: () => jest.fn() + } + }; let renderSideNavHook = (menuProps) => { - let {result} = renderHook(() => useSideNav({...menuProps, layout: mockLayout}, mockState)); + let {result} = renderHook(() => useSideNav({...menuProps, layout: mockLayout}, mockState, mockRef)); return result.current; }; From bc45c58026541399f3c8bb9a1b6bdc039bf0bade Mon Sep 17 00:00:00 2001 From: Daniel Lu Date: Fri, 10 Sep 2021 11:55:54 -0700 Subject: [PATCH 4/4] whoops extra space --- packages/@react-aria/sidenav/test/useSideNav.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/sidenav/test/useSideNav.test.js b/packages/@react-aria/sidenav/test/useSideNav.test.js index 252c6ca00e9..7eba4ea7208 100644 --- a/packages/@react-aria/sidenav/test/useSideNav.test.js +++ b/packages/@react-aria/sidenav/test/useSideNav.test.js @@ -28,7 +28,7 @@ describe('useSideNav', function () { }; let renderSideNavHook = (menuProps) => { - let {result} = renderHook(() => useSideNav({...menuProps, layout: mockLayout}, mockState, mockRef)); + let {result} = renderHook(() => useSideNav({...menuProps, layout: mockLayout}, mockState, mockRef)); return result.current; };