Repository navigation
fix(tooltip): keep Kumo's disabled trigger out of the tab order - #902
Merged
mattrothenberg merged 1 commit intoOct 28, 2026
Merged
mattrothenberg merged 1 commit into
mattrothenberg merged 1 commit into
Conversation
wking-io
marked this pull request as ready for review
October 28, 2026 20:55
|
This pull request was not reviewed automatically because its author does not have write access to this repository. A maintainer with write access can start a review by commenting |
When Kumo renders its own trigger (no render or asChild), it wraps non-interactive content such as truncated Text that only needs a tooltip when it overflows. A disabled tooltip there has nothing to reveal, but the native button trigger was still a tab stop. Set tabIndex -1 on Kumo's trigger while disabled; custom triggers keep their own focus behavior. Co-authored-by: Pi <noreply@pi.dev> Co-authored-by: cloudflare <noreply@unknown.com> Co-authored-by: claude-opus-5-5 <noreply@unknown.com>
wking-io
force-pushed
the
wking/tooltip-disabled-trigger-tab-order
branch
from
October 28, 2026 21:33
e5932e7 to
186fc21
Compare
commit: |
Contributor
Docs PreviewCommit: |
Contributor
Visual contractsLinux Chromium references exercised by the PR's 1 curated contracttruncating text trigger
|
mattrothenberg
approved these changes
Oct 28, 2026
Contributor
📐 Bundle SizeConsumer fixtures bundled against
npm tarball: 565 files, 1.56 MB packed, 7.30 MB unpacked.
|
Contributor
Visual Regression Report — 0 changed, 6 unchangedNo visual changes detected. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When
Tooltiprenders its own trigger (norenderorasChild), the trigger is a native<button>around non-interactive content. The common case is truncatedTextthat needs a tooltip only when it overflows:While the tooltip is
disabled, the trigger has nothing to reveal, but it stays a tab stop. In a table of names, keyboard users stop on every row even though only the truncated ones show anything. Consumers can't fix this themselves, becauseTooltippasses extra props toTooltip.Root, not the trigger. Their workaround is a customrendertrigger with a hand-writtentabIndex. But a custom trigger skips Kumo's defensive trigger resets (border-none bg-transparent p-0 …), so in apps without Tailwind preflight it picks up native button styling unless the consumer copies those resets too.Fix
Kumo's own trigger gets
tabIndex={-1}while the tooltip isdisabled. Nothing else changes:renderor the deprecatedasChild) keep their own focus behavior. They're usually real controls;Sidebar.MenuButtonandMenubarboth userender, so they're unaffected.With this, the snippet above works as-is: truncated names are focusable and show the tooltip, and names that fit aren't tab stops.
Behavior note
Anyone relying on a disabled tooltip's default trigger being keyboard-focusable will see it skipped. That seems unlikely to be intentional, since the trigger does nothing while disabled, but say so if you know of a case. A separate
focusable-style option would avoid it, at the cost of new API.Tests
Three unit tests in
tooltip.test.tsx. Each tabs from a button before the tooltip:tabindex="-1", and focus skips to the next button. Fails onmain, passes with this change.tabindex, and the trigger takes focus. Passes on both.rendertrigger: notabindex, and the trigger takes focus. Passes on both.Checks:
Unit suite: 1315 passed, 0 failed (after
codegen:demosandcodegen:registry).pnpm typecheck,pnpm lintand Oxfmt are clean.The tooltip visual contract ("keeps a truncating Text trigger visible") renders an enabled tooltip, so this change doesn't reach it. Locally on macOS it fails with an image size mismatch (78×20 vs the stored 148×38), and
mainfails the same way.Reviews
bonk has reviewed the change
automated review not possible because: bonk requires repository write access, which this fork's author doesn't have yet; requesting human review
Tests
Tests included/updated
Automated tests not possible - manual testing has been completed as follows:
Additional testing not necessary because: