From 8f9fddc2f09df24a0735f318f80980b2e03e51ff Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Wed, 3 Aug 2022 21:24:49 -0700 Subject: [PATCH] Stop propagation when pressing Enter on a native link --- .../@react-aria/interactions/src/usePress.ts | 21 ++++---- .../@react-spectrum/table/test/Table.test.js | 48 +++++++++++++++++++ 2 files changed, 61 insertions(+), 8 deletions(-) diff --git a/packages/@react-aria/interactions/src/usePress.ts b/packages/@react-aria/interactions/src/usePress.ts index 5810c407b72..64bd621d1d5 100644 --- a/packages/@react-aria/interactions/src/usePress.ts +++ b/packages/@react-aria/interactions/src/usePress.ts @@ -228,7 +228,7 @@ export function usePress(props: PressHookProps): PressResult { let pressProps: DOMAttributes = { onKeyDown(e) { - if (isValidKeyboardEvent(e.nativeEvent) && e.currentTarget.contains(e.target as Element)) { + if (isValidKeyboardEvent(e.nativeEvent, e.currentTarget) && e.currentTarget.contains(e.target as Element)) { if (shouldPreventDefaultKeyboard(e.target as Element)) { e.preventDefault(); } @@ -246,10 +246,15 @@ export function usePress(props: PressHookProps): PressResult { // instead of the same element where the key down event occurred. addGlobalListener(document, 'keyup', onKeyUp, false); } + } else if (e.key === 'Enter' && isHTMLAnchorLink(e.currentTarget)) { + // If the target is a link, we won't have handled this above because we want the default + // browser behavior to open the link when pressing Enter. But we still need to prevent + // default so that elements above do not also handle it (e.g. table row). + e.stopPropagation(); } }, onKeyUp(e) { - if (isValidKeyboardEvent(e.nativeEvent) && !e.repeat && e.currentTarget.contains(e.target as Element)) { + if (isValidKeyboardEvent(e.nativeEvent, e.currentTarget) && !e.repeat && e.currentTarget.contains(e.target as Element)) { triggerPressUp(createEvent(state.target, e), 'keyboard'); } }, @@ -284,7 +289,7 @@ export function usePress(props: PressHookProps): PressResult { }; let onKeyUp = (e: KeyboardEvent) => { - if (state.isPressed && isValidKeyboardEvent(e)) { + if (state.isPressed && isValidKeyboardEvent(e, state.target)) { if (shouldPreventDefaultKeyboard(e.target as Element)) { e.preventDefault(); } @@ -297,7 +302,7 @@ export function usePress(props: PressHookProps): PressResult { // If the target is a link, trigger the click method to open the URL, // but defer triggering pressEnd until onClick event handler. - if (state.target instanceof HTMLElement && (state.target.contains(target) && isHTMLAnchorLink(state.target) || state.target.getAttribute('role') === 'link')) { + if (state.target instanceof HTMLElement && state.target.contains(target) && (isHTMLAnchorLink(state.target) || state.target.getAttribute('role') === 'link')) { state.target.click(); } } @@ -662,13 +667,13 @@ export function usePress(props: PressHookProps): PressResult { }; } -function isHTMLAnchorLink(target: HTMLElement): boolean { +function isHTMLAnchorLink(target: Element): boolean { return target.tagName === 'A' && target.hasAttribute('href'); } -function isValidKeyboardEvent(event: KeyboardEvent): boolean { - const {key, code, target} = event; - const element = target as HTMLElement; +function isValidKeyboardEvent(event: KeyboardEvent, currentTarget: Element): boolean { + const {key, code} = event; + const element = currentTarget as HTMLElement; const {tagName, isContentEditable} = element; const role = element.getAttribute('role'); // Accessibility for keyboards. Space and Enter only. diff --git a/packages/@react-spectrum/table/test/Table.test.js b/packages/@react-spectrum/table/test/Table.test.js index c91f866f78b..ee2bfe09864 100644 --- a/packages/@react-spectrum/table/test/Table.test.js +++ b/packages/@react-spectrum/table/test/Table.test.js @@ -1822,6 +1822,54 @@ describe('TableView', function () { let checkbox = tree.getByLabelText('Select All'); expect(checkbox.checked).toBeFalsy(); }); + + describe('Space key with focus on a link within a cell', () => { + it('should toggle selection and prevent scrolling of the table', () => { + let tree = render( + + + {column => {column.name}} + + + {item => + ( + {key => {item[key]}} + ) + } + + + ); + + let row = tree.getAllByRole('row')[1]; + expect(row).toHaveAttribute('aria-selected', 'false'); + + let link = within(row).getAllByRole('link')[0]; + expect(link.textContent).toBe('Foo 1'); + + act(() => { + link.focus(); + fireEvent.keyDown(link, {key: ' '}); + fireEvent.keyUp(link, {key: ' '}); + jest.runAllTimers(); + }); + + row = tree.getAllByRole('row')[1]; + expect(row).toHaveAttribute('aria-selected', 'true'); + + act(() => { + link.focus(); + fireEvent.keyDown(link, {key: ' '}); + fireEvent.keyUp(link, {key: ' '}); + jest.runAllTimers(); + }); + + row = tree.getAllByRole('row')[1]; + link = within(row).getAllByRole('link')[0]; + + expect(row).toHaveAttribute('aria-selected', 'false'); + expect(link.textContent).toBe('Foo 1'); + }); + }); }); describe('range selection', function () {