[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
from
048b73d to
e59ac6f
Compare
mj12albert
force-pushed
the
fix/tab-mouse-button
branch
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?
| onTabActivation(tabValue, event.nativeEvent); | ||
| }, | ||
| onFocus(event) { |
There was a problem hiding this comment.
I'm confused. Why does the tab activate on mouse down? I thought this point of using onClick is cancellability. Having to use activateOnFocus to get this behavior feels wrong, this is a different concern.
I would see those as correct:
- https://headlessui.com/react/tabs
- https://mui.com/base-ui/react-tabs/
- https://www.w3.org/WAI/ARIA/apg/patterns/tabs/
- https://ariakit.org/components/tab
those as wrong:
- https://www.radix-ui.com/primitives/docs/components/tabs
- https://react-spectrum.adobe.com/react-aria/Tabs.html
Screen.Recording.2025-01-10.at.23.56.37.mov
But then maybe it's OK if we assume this: applications that have a significant UI state behind tabs must use <a> to trigger the actions. a link triggering on onClick, means it's OK. And then if they don't use a <a>, it means that the state is not that important, so maybe mouseDown is better. I don't really buy that narrative though to be honest, e.g. navigating inside Ashby's UI.
There was a problem hiding this comment.
John Carmack had a good argument for acting on press wherever possible in that the UI feels insanely faster and more responsive. I've switched most things to act on press (mouse/key down) and it is 10000% true. Some tabbed form component for example had some quite slow tabs since it was quite complex and this hid its slowness.
Fixes #1317