fix(menu): Safari issue on closing menu clicking on trigger #2338
Conversation
🦋 Changeset detectedLatest commit: ff399fb The changes in this PR will be included in the next version bump. This PR includes changesets to release 33 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
size-limit report 📦
|
|
Can you explain your considerations around the Downshift approach a bit more? Looking at the code I think I understand it better to be honest. |
| @@ -150,12 +150,26 @@ export function Menu(props: MenuProps) { | |||
| [closeAndFocusTrigger, handleArrowsKeyDown], | |||
| ); | |||
|
|
|||
| const isMouseDown = useRef<Boolean>(false); | |||
There was a problem hiding this comment.
| const isMouseDown = useRef<Boolean>(false); | |
| // the comment that explains the issue in safari that we are trying to fix here. Because the one we have in the `onMouseDown` event is not clear enough for me. | |
| const isTriggerMouseDown = useRef<Boolean>(false); |
WDYT?
| // handle odd case for Safari which | ||
| // prevent triggering blur when MouseDown | ||
| isMouseDown.current = true; | ||
| _props.onMouseDown?.(event); | ||
| }, | ||
| onMouseUp: (event) => { | ||
| // handle odd case for Safari | ||
| // set isMouseDown as false to enable blur |
There was a problem hiding this comment.
and then this comments we can delete I guess
Sorry, had a typo there, I fixed it. |
Purpose of PR
Fix issue with Safari when trying to close menu by clicking on the trigger
Obs:
tabindex=0inside the button and use it to trigger the events, which in our case would be tricky since the trigger could be a button or not.MouseDown, so trying to set the focus on theonClickwasn't working. Which would means that we would have to manually focus on the events.