adobe / adobe/react-spectrum

Have usePress call createEventHandler for onKeyDown

Open
#2,433 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
15.9k
Forks
1.6k
Avg merge
3d 9m
Merged PRs (30d)
59

Description

# 🐛 Bug Report

## 🤔 Expected Behavior
Note: TBH, I'm not entirely sure if this is a problem, just felt inconsistent to me.

Surfaced from discussions around https://github.com/adobe/react-spectrum/pull/2354. The reason the issue in that PR doesn't happen for our spectrum components (i.e. MenuTrigger) is because the [menuTrigger keydown handlers](https://github.com/adobe/react-spectrum/blob/main/packages/@react-spectrum/menu/src/MenuTrigger.tsx#L44) are sent directly via [context](https://github.com/adobe/react-spectrum/blob/main/packages/@react-spectrum/menu/src/MenuTrigger.tsx#L117) to useButton's [usePress](https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/button/src/useButton.ts#L83) and don't get sent to useButton's [useFocusable](https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/button/src/useButton.ts#L93) call. This means the [stopPropagation](https://github.com/adobe/react-spectrum/blob/b4a3821acce160a0013587b6891e864390a33efe/packages/%40react-aria/menu/src/useMenuTrigger.ts#L59) call from useMenuTrigger doesn't trigger the warning in [createEventHandler](https://github.com/adobe/react-spectrum/blob/main/packages/@react-aria/interactions/src/createEventHandler.ts#L34-L36), which from a behavioral standpoint is fine but it feels odd to sidestep this issue just because usePress doesn't use createEventHandler to stop propagation by default.

## 😯 Current Behavior

The useMenuTrigger aria example causes a console.error when doing ArrowUp/Down etc but MenuTrigger doesn't

## 💁 Possible Solution

Call createEventHandler on the onKeyDown handler generated inside usePress. Perhaps place it before the return [here](https://github.com/adobe/react-spectrum/blob/b4a3821acce160a0013587b6891e864390a33efe/packages/%40react-aria/interactions/src/usePress.ts#L626-L630).
Can then remove https://github.com/adobe/react-spectrum/blob/b4a3821acce160a0013587b6891e864390a33efe/packages/%40react-aria/interactions/src/usePress.ts#L228

| Software | Version(s) |
| ---------------- | ---------- |
| react-spectrum | 3.14.1
| Browser |
| Operating System |

## 🧢 Your Company/Team

RSP

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.