From 327cdc4dfc6b8679746d8c2a1f4ad24d49b9f70d Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Fri, 23 May 2025 16:03:24 -0700 Subject: [PATCH 1/4] fix: stop combobox from closing when keyboard navigating --- .../selection/src/useSelectableCollection.ts | 12 ++++++------ packages/@react-spectrum/tag/test/TagGroup.test.js | 3 ++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index a85fdecb54d..0e74382f8bc 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -505,7 +505,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions // Scroll the focused element into view when the focusedKey changes. let lastFocusedKey = useRef(manager.focusedKey); - let raf = useRef(null); + let raf = useRef>(undefined); useEffect(() => { if (manager.isFocused && manager.focusedKey != null && (manager.focusedKey !== lastFocusedKey.current || didAutoFocusRef.current) && scrollRef.current && ref.current) { let modality = getInteractionModality(); @@ -525,12 +525,12 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions raf.current = requestAnimationFrame(() => { if (scrollRef.current) { scrollIntoView(scrollRef.current, element); + // Avoid scroll in iOS VO, since it may cause overlay to close (i.e. RAC submenu) + if (modality !== 'virtual') { + scrollIntoViewport(element, {containingElement: ref.current}); + } } }); - // Avoid scroll in iOS VO, since it may cause overlay to close (i.e. RAC submenu) - if (modality !== 'virtual') { - scrollIntoViewport(element, {containingElement: ref.current}); - } } } @@ -541,7 +541,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions lastFocusedKey.current = manager.focusedKey; didAutoFocusRef.current = false; - }); + }, [manager.focusedKey, manager.isFocused, ref, scrollRef]); useEffect(() => { return () => { diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index f8bb0198bf9..994d3d8fad8 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -608,7 +608,7 @@ describe('TagGroup', function () { .mockImplementationOnce(() => ({x: 200, y: 400, width: 95, height: 32, top: 400, right: 290, bottom: 435, left: 200})) .mockImplementationOnce(() => ({x: 200, y: 300, width: 200, height: 128, top: 300, right: 400, bottom: 435, left: 200})) .mockImplementationOnce(() => ({x: 265, y: 335, width: 75, height: 32, top: 335, right: 345, bottom: 370, left: 265})) - .mockImplementationOnce(() => ({x: 200, y: 300, width: 75, height: 32, top: 300, right: 275, bottom: 335, left: 200})); + .mockImplementationOnce(() => ({x: 200, y: 300, width: 75, height: 32, top: 300, right: 275, bottom: 335, left: 200})) let {getAllByRole} = render( jest.runAllTimers()); expect(document.activeElement).toBe(tags[0]); await user.tab(); From 523b34964df7a34ef07d96936b3b2f88ba65b324 Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Fri, 23 May 2025 16:04:41 -0700 Subject: [PATCH 2/4] fix lint --- packages/@react-spectrum/tag/test/TagGroup.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-spectrum/tag/test/TagGroup.test.js b/packages/@react-spectrum/tag/test/TagGroup.test.js index 994d3d8fad8..567cd711cd8 100644 --- a/packages/@react-spectrum/tag/test/TagGroup.test.js +++ b/packages/@react-spectrum/tag/test/TagGroup.test.js @@ -608,7 +608,7 @@ describe('TagGroup', function () { .mockImplementationOnce(() => ({x: 200, y: 400, width: 95, height: 32, top: 400, right: 290, bottom: 435, left: 200})) .mockImplementationOnce(() => ({x: 200, y: 300, width: 200, height: 128, top: 300, right: 400, bottom: 435, left: 200})) .mockImplementationOnce(() => ({x: 265, y: 335, width: 75, height: 32, top: 335, right: 345, bottom: 370, left: 265})) - .mockImplementationOnce(() => ({x: 200, y: 300, width: 75, height: 32, top: 300, right: 275, bottom: 335, left: 200})) + .mockImplementationOnce(() => ({x: 200, y: 300, width: 75, height: 32, top: 300, right: 275, bottom: 335, left: 200})); let {getAllByRole} = render( Date: Fri, 23 May 2025 16:06:05 -0700 Subject: [PATCH 3/4] remove dependencies --- packages/@react-aria/selection/src/useSelectableCollection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 0e74382f8bc..7782dff5483 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -541,7 +541,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions lastFocusedKey.current = manager.focusedKey; didAutoFocusRef.current = false; - }, [manager.focusedKey, manager.isFocused, ref, scrollRef]); + }); useEffect(() => { return () => { From 0e6389f3ccfef5faeb291c464d9cb5fb88e3391d Mon Sep 17 00:00:00 2001 From: Yihui Liao <44729383+yihuiliao@users.noreply.github.com> Date: Tue, 27 May 2025 10:17:41 -0700 Subject: [PATCH 4/4] update type --- packages/@react-aria/selection/src/useSelectableCollection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 7782dff5483..317d6e48873 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -505,7 +505,7 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions // Scroll the focused element into view when the focusedKey changes. let lastFocusedKey = useRef(manager.focusedKey); - let raf = useRef>(undefined); + let raf = useRef(null); useEffect(() => { if (manager.isFocused && manager.focusedKey != null && (manager.focusedKey !== lastFocusedKey.current || didAutoFocusRef.current) && scrollRef.current && ref.current) { let modality = getInteractionModality();