Skip to content

fix(web): prevent MenuButton from opening on focus loss#2345

Merged
dgdavid merged 2 commits intomasterfrom
fix-menu-button-onOpenChange
May 8, 2025
Merged

fix(web): prevent MenuButton from opening on focus loss#2345
dgdavid merged 2 commits intomasterfrom
fix-menu-button-onOpenChange

Conversation

@dgdavid
Copy link
Contributor

@dgdavid dgdavid commented May 8, 2025

Avoid reusing the toggle callback from MenuToggle#onClick as the MenuContainer#onOpenChange handler. These two callbacks serve different purposes:

  • toggle is intended to handle events and receives a SyntheticEvent.
  • onOpenChange is triggered by focus-related events and receives a boolean indicating the next isOpen state.

Using toggle for onOpenChange led to incorrect behavior, such as the menu opening when focus was lost.

Screencasts

Before

Screencast.From.2025-05-08.14-41-13.mp4

After

Screencast.From.2025-05-08.14-40-38.mp4

Avoid reusing the `toggle` callback from `MenuToggle#onClick` as the
`MenuContainer#onOpenChange` handler. These two callbacks serve
different purposes:
  - `toggle` is intended to handle click events and receives a `SyntheticEvent`.
  - `onOpenChange` is triggered by focus-related events and expects a
    boolean indicating the next `isOpen` state.

Using `toggle` for `onOpenChange` led to incorrect behavior, such as the
menu opening when focus was lost.
@dgdavid dgdavid force-pushed the fix-menu-button-onOpenChange branch from 3c66d64 to 464f3a0 Compare May 8, 2025 13:43
@dgdavid dgdavid requested review from ancorgs and joseivanlopez May 8, 2025 13:45
Copy link
Contributor

@ancorgs ancorgs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dgdavid dgdavid merged commit e461d55 into master May 8, 2025
7 checks passed
@dgdavid dgdavid deleted the fix-menu-button-onOpenChange branch May 8, 2025 14:05
@imobachgs imobachgs mentioned this pull request May 26, 2025
imobachgs added a commit that referenced this pull request May 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants