[Tabs] Fix tabs activating incorrectly on non-primary button clicks #1318
Conversation
mj12albert
added
component: tabs
✅ Deploy Preview for base-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
mj12albert
force-pushed
the
fix/tab-mouse-button
branch
2 times, most recently
from
e59ac6f to
504cda3
Compare
mj12albert
requested review from
atomiks,
colmtuite and
michaldudak
as code owners
| isPrimaryButtonRef.current = false; | ||
| } | ||
|
|
||
| if (!event.button || event.button === 0) { |
There was a problem hiding this comment.
We use
| if (!event.button || event.button === 0) { | |
| if (event.button === 0) { |
in all Material UI codebase. I have never seen someone open a bug because event.button is missing. Could this reduce the bundle size here?
There was a problem hiding this comment.
Tried removing this check in NumberField here too but without it a test fails: https://app.circleci.com/pipelines/github/mui/base-ui/7042/workflows/cc6f84c4-5cc2-488c-b741-ebb30c275d4a/jobs/61996/parallel-runs/0/steps/0-106
Not sure if it's just a test issue but I'd prefer to keep the main button check the same for now CC @atomiks
mj12albert
force-pushed
the
fix/tab-mouse-button
branch
from
504cda3 to
6e63c2b
Compare
| </Tabs.Root>, | ||
| ); | ||
|
|
||
| await user.pointer({ keys: '[MouseLeft>]', target: getAllByRole('tab')[1] }); |
There was a problem hiding this comment.
https://testing-library.com/docs/user-event/pointer/#pressing-a-button-or-touching-the-screen
pressing and releasing, also works for keyboard 😬 user.keyboard('{a>}') // press a without releasing it
Fixes #1317