fix(#2599): TableView: space key on link within cell scrolls the TableView down a page

[majornista]

Closes #2599

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue 2559.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

1. Open https://reactspectrum.blob.core.windows.net/reactspectrum/903179155a85c47707c9bfb6902e0deef1b7d892/storybook/index.html?path=/story/tableview--async-loading
2. Tab to move focus to the first article link in the first row within the table body.
3. Press Space key.
4. Since this is a link, Space shouldn't activate the link, but the event should bubble to be handled by the table row, toggling selection. preventDefault should be called on the event to prevent the TableView from scrolling.

🧢 Your Project:

Adobe/Accessibility

[ktabors]

I tested the table with the link and it worked. Nervous about this causing regressions.

[dkario]

The code looks good. I defer to RSP teammates on potential regressions

[devongovett]

This is a bit more changes than I was hoping. Should we perhaps use e.currentTarget in isValidKeyboardEvent rather than e.target? I think the problem is that the usePress on the link correctly ignores the event, but the table row also ignores it when it shouldn't due to the target still being the link. If we used currentTarget, this would not happen because the target would be the row element. currentTarget wouldn't work for the global keyup listener that we have, so perhaps pass the target in as an argument to isValidKeyboardEvent and use state.target in that case.

[majornista]

This is a bit more changes than I was hoping. Should we perhaps use e.currentTarget in isValidKeyboardEvent rather than e.target? I think the problem is that the usePress on the link correctly ignores the event, but the table row also ignores it when it shouldn't due to the target still being the link. If we used currentTarget, this would not happen because the target would be the row element. currentTarget wouldn't work for the global keyup listener that we have, so perhaps pass the target in as an argument to isValidKeyboardEvent and use state.target in that case.

currentTarget doesn’t exist on the nativeEvent when the keyboard event fires from the link.

[majornista]

This is a bit more changes than I was hoping. Should we perhaps use e.currentTarget in isValidKeyboardEvent rather than e.target? I think the problem is that the usePress on the link correctly ignores the event, but the table row also ignores it when it shouldn't due to the target still being the link. If we used currentTarget, this would not happen because the target would be the row element. currentTarget wouldn't work for the global keyup listener that we have, so perhaps pass the target in as an argument to isValidKeyboardEvent and use state.target in that case.

@devongovett See 44909d5, which I think consolidates the logic for a link within an ancestor that also handles the space key to trigger a usePress into isValidKeyboardEvent.

[adobe-bot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[adobe-bot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[adobe-bot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation

[adobe-bot]

Build successful! 🎉

• View the storybook
• View the storybook-17
• View the storybook-16
• View the documentation