Repository navigation
fix(Tabs): don't activate tabs on right or middle click - #3027
Conversation
The mousedown handler used the `.left` modifier, so for other buttons it never ran and never prevented the focus, and a focused tab was activated in automatic mode. Run it for every button and only activate on the left one, as Radix does. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Restrict activation to left button packages/core/src/Tabs/TabsTrigger.vue, packages/core/src/Tabs/Tabs.test.ts, packages/core/src/Tabs/Tabs.browser.test.ts |
The mousedown handler checks for event.button === 0. Unit and browser tests verify left-button activation and that middle- and right-button events do not activate or focus a tab. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Bug fix · Severity of issue fixed: Low
Merge Risk: 
Minimal · up to e5425
-
The supplied evidence identifies no issue that needs resolution before merging.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed-
Full details: Title check
-
Explanation
The title "fix(Tabs): don't activate tabs on right or middle click" matches the main change. The changeset makes TabsTrigger activate only on left-button mousedown (event.button === 0) and prevents default for other buttons, so right and middle clicks no longer activate or focus a tab. The title is short, specific, and names the affected component. It follows a conventional commit style. The objectives describe the same change, and the linked issue
#2258asks for the same behavior.
Full details: Linked Issues check
-
Explanation
[
#2258] requires that tab selection change only for the left mouse button. At the reviewed head,TabsTrigger.vuelistens to@mousedownfor all buttons. It callsrootContext.changeModelValue(value)only when!disabled && event.button === 0 && event.ctrlKey === false. Every other case reaches theelsebranch, which callsevent.preventDefault(). This stops focus, so the@focusautomatic-activation path does not select the tab. The change is in the whole-PR diff against the merge base.Tabs.test.tsadds a case for buttons 1 and 2. It checks thatvaluestays'one'and thatevent.defaultPreventedistrue. Under the previous@mousedown.leftlistener, a button 1 or 2 event would not reach the handler, so this test checks the fixed behavior.Tabs.browser.test.tsadds real-browser tests for right and middle clicks. They check that the second tab is not focused or activated. No coding requirement from#2258is unmet. The PR description states that thessr hydrationtest fails onv2in the author's environment. This is outside the scope of this PR. Thev3branch is not part of this review.
Full details: Out of Scope Changes check
-
Explanation
The whole-PR diff changes three files.
packages/core/src/Tabs/TabsTrigger.vuechanges the mousedown handler:.leftis replaced by an explicitevent.button === 0check. Existing disabled and Ctrl checks remain.packages/core/src/Tabs/Tabs.test.tsadds a unit test for buttons 1 and 2.packages/core/src/Tabs/Tabs.browser.test.tsis a new file with real-browser tests for right and middle clicks onTabsTrigger. Each change supports the fix for#2258. No unrelated component, public API, or behavior change appears in the diff. Theelsebranch that prevents default for non-left clicks also applies to disabled triggers. Those triggers previously reached the same branch for left clicks, so their behavior does not change.
-
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
…browser jsdom never moves focus on mousedown, so the focus-driven activation from unovue#2258 only reproduces in Chromium. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
commit: |
🔗 Linked issue
Resolves #2258
❓ Type of change
📚 Description
TabsTriggerlistened with@mousedown.left, so its handler never ran for the middle or right button. Theelsebranch that prevents focus "to avoid accidental activation" was therefore never reached for those buttons: the tab got focus, and in the default automatic activation mode,@focusthen selected it.The listener now runs for every button, and only the left button (without Ctrl) activates the tab; other buttons prevent the default, so the tab doesn't take focus. This matches Radix's
TabsTrigger, which this component follows.The new tests dispatch a
mousedownwith button 1 and 2 and check that the tab isn't activated and the event's default is prevented. They fail without this change. The other Tabs tests pass, except "ssr hydration", which fails the same way onv2in my environment.The
v3branch'suseTabs.tshas the same logic; I can open a follow-up there if this approach is fine.I used Claude Code to write this change.
📝 Checklist
Summary by CodeRabbit