From c4a783e893eca8043a386283ff7074db46c5fefc Mon Sep 17 00:00:00 2001 From: Medhashis Maiti <129734281+mdhmaiti@users.noreply.github.com> Date: Wed, 14 Aug 2024 05:25:52 +0530 Subject: [PATCH] fix/bug useTablist #5996 (#6023) * fix/bug useTabist #5996 and added tests --- .../selection/src/useSelectableCollection.ts | 44 ++++++++++++------- .../tabs/src/TabsKeyboardDelegate.ts | 23 +++++++--- .../@react-spectrum/tabs/test/Tabs.test.js | 23 ++++++---- 3 files changed, 59 insertions(+), 31 deletions(-) diff --git a/packages/@react-aria/selection/src/useSelectableCollection.ts b/packages/@react-aria/selection/src/useSelectableCollection.ts index 912ffc04099..4bb27efd2e4 100644 --- a/packages/@react-aria/selection/src/useSelectableCollection.ts +++ b/packages/@react-aria/selection/src/useSelectableCollection.ts @@ -164,49 +164,57 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions switch (e.key) { case 'ArrowDown': { if (delegate.getKeyBelow) { - e.preventDefault(); let nextKey = manager.focusedKey != null - ? delegate.getKeyBelow(manager.focusedKey) + ? delegate.getKeyBelow?.(manager.focusedKey) : delegate.getFirstKey?.(); if (nextKey == null && shouldFocusWrap) { nextKey = delegate.getFirstKey?.(manager.focusedKey); } - navigateToKey(nextKey); + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey); + } } break; } case 'ArrowUp': { if (delegate.getKeyAbove) { - e.preventDefault(); let nextKey = manager.focusedKey != null - ? delegate.getKeyAbove(manager.focusedKey) + ? delegate.getKeyAbove?.(manager.focusedKey) : delegate.getLastKey?.(); if (nextKey == null && shouldFocusWrap) { nextKey = delegate.getLastKey?.(manager.focusedKey); } - navigateToKey(nextKey); + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey); + } } break; } case 'ArrowLeft': { if (delegate.getKeyLeftOf) { - e.preventDefault(); - let nextKey = delegate.getKeyLeftOf(manager.focusedKey); + let nextKey = delegate.getKeyLeftOf?.(manager.focusedKey); if (nextKey == null && shouldFocusWrap) { nextKey = direction === 'rtl' ? delegate.getFirstKey?.(manager.focusedKey) : delegate.getLastKey?.(manager.focusedKey); } - navigateToKey(nextKey, direction === 'rtl' ? 'first' : 'last'); + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey, direction === 'rtl' ? 'first' : 'last'); + } } break; } case 'ArrowRight': { if (delegate.getKeyRightOf) { - e.preventDefault(); - let nextKey = delegate.getKeyRightOf(manager.focusedKey); + let nextKey = delegate.getKeyRightOf?.(manager.focusedKey); if (nextKey == null && shouldFocusWrap) { nextKey = direction === 'rtl' ? delegate.getLastKey?.(manager.focusedKey) : delegate.getFirstKey?.(manager.focusedKey); } - navigateToKey(nextKey, direction === 'rtl' ? 'last' : 'first'); + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey, direction === 'rtl' ? 'last' : 'first'); + } } break; } @@ -236,16 +244,20 @@ export function useSelectableCollection(options: AriaSelectableCollectionOptions break; case 'PageDown': if (delegate.getKeyPageBelow) { - e.preventDefault(); let nextKey = delegate.getKeyPageBelow(manager.focusedKey); - navigateToKey(nextKey); + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey); + } } break; case 'PageUp': if (delegate.getKeyPageAbove) { - e.preventDefault(); let nextKey = delegate.getKeyPageAbove(manager.focusedKey); - navigateToKey(nextKey); + if (nextKey != null) { + e.preventDefault(); + navigateToKey(nextKey); + } } break; case 'a': diff --git a/packages/@react-aria/tabs/src/TabsKeyboardDelegate.ts b/packages/@react-aria/tabs/src/TabsKeyboardDelegate.ts index 6ce58947848..463bad748f5 100644 --- a/packages/@react-aria/tabs/src/TabsKeyboardDelegate.ts +++ b/packages/@react-aria/tabs/src/TabsKeyboardDelegate.ts @@ -16,11 +16,13 @@ export class TabsKeyboardDelegate implements KeyboardDelegate { private collection: Collection>; private flipDirection: boolean; private disabledKeys: Set; + private tabDirection: boolean; constructor(collection: Collection>, direction: Direction, orientation: Orientation, disabledKeys: Set = new Set()) { this.collection = collection; this.flipDirection = direction === 'rtl' && orientation === 'horizontal'; this.disabledKeys = disabledKeys; + this.tabDirection = orientation === 'horizontal'; } getKeyLeftOf(key: Key) { @@ -37,13 +39,6 @@ export class TabsKeyboardDelegate implements KeyboardDelegate { return this.getNextKey(key); } - getKeyAbove(key: Key) { - return this.getPreviousKey(key); - } - - getKeyBelow(key: Key) { - return this.getNextKey(key); - } private isDisabled(key: Key) { return this.disabledKeys.has(key) || !!this.collection.getItem(key)?.props?.isDisabled; @@ -64,6 +59,20 @@ export class TabsKeyboardDelegate implements KeyboardDelegate { } return key; } + + getKeyAbove(key: Key) { + if (this.tabDirection) { + return null; + } + return this.getPreviousKey(key); + } + + getKeyBelow(key: Key) { + if (this.tabDirection) { + return null; + } + return this.getNextKey(key); + } getNextKey(key) { do { diff --git a/packages/@react-spectrum/tabs/test/Tabs.test.js b/packages/@react-spectrum/tabs/test/Tabs.test.js index e402a0995ad..7a411b22046 100644 --- a/packages/@react-spectrum/tabs/test/Tabs.test.js +++ b/packages/@react-spectrum/tabs/test/Tabs.test.js @@ -10,7 +10,7 @@ * governing permissions and limitations under the License. */ -import {act, fireEvent, mockImplementation, pointerMap, render, waitFor, within} from '@react-spectrum/test-utils-internal'; +import {act, createEvent, fireEvent, mockImplementation, pointerMap, render, waitFor, within} from '@react-spectrum/test-utils-internal'; import {Item, TabList, TabPanels, Tabs} from '../src'; import {Links as LinksExample} from '../stories/Tabs.stories'; import {Provider} from '@react-spectrum/provider'; @@ -113,18 +113,25 @@ describe('Tabs', function () { expect(selectedItem).toHaveAttribute('aria-selected', 'true'); act(() => {selectedItem.focus();}); - fireEvent.keyDown(selectedItem, {key: 'ArrowRight', code: 39, charCode: 39}); + let arrowRight = createEvent.keyDown(selectedItem, {key: 'ArrowRight', code: 39, charCode: 39}); + fireEvent(selectedItem, arrowRight); let nextSelectedItem = tabs[1]; expect(nextSelectedItem).toHaveAttribute('aria-selected', 'true'); - fireEvent.keyDown(nextSelectedItem, {key: 'ArrowLeft', code: 37, charCode: 37}); + expect(arrowRight.defaultPrevented).toBe(true); + let arrowLeft = createEvent.keyDown(nextSelectedItem, {key: 'ArrowLeft', code: 37, charCode: 37}); + fireEvent(nextSelectedItem, arrowLeft); expect(selectedItem).toHaveAttribute('aria-selected', 'true'); + expect(arrowLeft.defaultPrevented).toBe(true); - /** Changes selection regardless if it's horizontal tabs. */ - fireEvent.keyDown(selectedItem, {key: 'ArrowUp', code: 38, charCode: 38}); - nextSelectedItem = tabs[2]; - expect(nextSelectedItem).toHaveAttribute('aria-selected', 'true'); - fireEvent.keyDown(selectedItem, {key: 'ArrowDown', code: 40, charCode: 40}); + /** prevent changing tabs for horizontal orientations in aria-selected */ + let arrowUp = createEvent.keyDown(selectedItem, {key: 'ArrowUp', code: 38, charCode: 38}); + fireEvent(selectedItem, arrowUp); + expect(selectedItem).toHaveAttribute('aria-selected', 'true'); + expect(arrowUp.defaultPrevented).toBe(false); + let arrowDown = createEvent.keyDown(selectedItem, {key: 'ArrowDown', code: 40, charCode: 40}); + fireEvent(selectedItem, arrowDown); expect(selectedItem).toHaveAttribute('aria-selected', 'true'); + expect(arrowDown.defaultPrevented).toBe(false); }); it('allows user to change tab item select via arrow keys with vertical tabs', function () {