fix(domains): the Remove Domain tooltip opens, so a disabled Remove says why

[dawsontoth]

⊙ Problem

On a cluster's Domains page, the tooltip on each row's Remove Domain button never opened. A bound domain's Remove is disabled, and nothing said why; an unbound domain never showed its DNS-cleanup reminder (#1843). The trigger slotted onto a fragment, which can take neither Radix's props nor its ref, so React dropped every handler and logged Invalid prop … supplied to React.Fragment on each render: 100 times in one Management.test.tsx run.

Stack: stage ← #1801 ← #1802 ← #1840 ← #1846 ← this PR. It is based on #1846's branch (claude/1686-domain-change-preflight) because that stack is rewriting this table. Retarget it once the stack lands.

❓ Your call: should this ride on the domains stack or wait for stage? I stacked it because #1846 edits the same file and test, so a separate stage PR would conflict. Moving it later only takes a rebase.

💡 Solution

The tooltip now triggers on the Remove control itself, and which element that is depends on whether the button can take focus. An enabled Remove is its own trigger, so hover and keyboard focus both open the DNS reminder, and Radix's aria-describedby lands on the element that has focus. A bound domain's Remove is disabled, and a disabled button takes no pointer events or focus, so a tabIndex={0} wrapper stands in as the trigger (the comment that says so). Mouse and keyboard users now get "To delete this domain, please unbind it from all clusters first." on a bound row, and "Remember to clean up your DNS records!" on an unbound one. The React.Fragment warning is gone from the test run.

The wrapper is keyed on the domain being bound, not on the button being disabled. While any domain change is in flight every Remove is disabled, and keying on that would give each unbound row a tab stop whose tip doesn't explain why.

❓ Your call: the bound-row wrapper is a focusable div with no role, so a screen reader announces its text and the tooltip but not "button, unavailable". The alternative is aria-disabled on the button with its click guarded in code, which keeps one element and its role. The wrapper is the pattern Radix documents for disabled triggers and stays local to this cell, so I kept it.

🔧 Changes

• src/features/cluster/domains/constants/tableDefinition.tsx: the Remove button is built once and handed to TooltipTrigger asChild either bare or inside the bound-row wrapper. The domain name moves out of the tooltip and stays above it, so the layout is unchanged.

❓ Your call: the original markup put the domain name inside the trigger too. I left it out, because hovering a name to read or copy it should not show a message about deleting it. Both outside review legs raised this. Putting the name back is a one-line move.

💬 gemini-code-assist feedback

The bot asked for a conditional tabIndex on the wrapper so keyboard users can reach a disabled Remove's tooltip. Fixed in f258657 as suggested, then reworked in f07861f after the pre-push round below found two problems with it: the description sat on the wrapper while an enabled button held focus, and keying on isPending added tab stops during any change.

✅ Verification

End-to-end route: component test through the real Radix tooltip, not a browser. Port 5173 is shared by parallel sessions here. Radix opens a tooltip from its trigger's onPointerMove and onFocus, and sets aria-describedby on that trigger while open (@radix-ui/react-tooltip/dist/index.mjs:186, :190, :208), and the repo's TooltipProvider defaults delayDuration to 0, so the test exercises the same handlers a mouse and a keyboard do. What a screen reader announces is not tested beyond the aria-describedby link.

• src/features/cluster/domains/Management.test.tsx, new the Remove Domain tooltip block, which finds the trigger by the data-state Radix puts on it and imports act to move focus:
  • for a bound domain and an unbound one, hovering the name opens nothing and hovering the trigger opens the matching message;
  • a disabled Remove's wrapper takes focus, and an enabled Remove is described on the button itself with no extra tab stop. Both check the focused element's aria-describedby against the tooltip's id;
  • a pending domain change adds no tab stop to an unbound row.
• npx vitest run src/features/cluster/domains/Management.test.tsx --silent=false: 53 passed, exit 0, 0 React.Fragment warnings. On the base branch it was 45 passed with 100 such warnings.
• Mutation checks at f07861f: the always-wrapper version fails the enabled-Remove test (aria-describedby was null) and the pending test; dropping the wrapper's tabIndex fails the disabled-focus test; keying the wrapper on isPending fails the pending test. At 58d0420, restoring the fragment trigger failed both hover cases, and putting the name back inside the trigger failed both.
• Pre-commit hook at f07861f: full vitest suite 399 files, 3746 passed, 11 skipped. npx tsc -b, npx oxlint --format stylish ., npx dprint check all exit 0.
• Cross-model review: two full rounds at 58d0420 ran codex (graded) and gemini. The Cursor legs failed both because the 1Password SSH agent refused their git fetch, and the Harper domain adjudicator failed both on an expired Claude OAuth session. Adopted then: narrowing the trigger to the button. A third round at f258657 ran codex, gemini and the Harper domain adjudicator. Adopted: describing an enabled Remove on the button, and not keying the wrapper on isPending (both in f07861f). Dropped by the adjudicator on evidence: "a 20ms wait can't prove the tooltip stayed closed", since every Tooltip mounts a provider with delayDuration 0 (src/components/ui/tooltip.tsx:9, :25). Kept: the one source comment, now cut to the constraint, because it is what stops someone from moving the trigger back onto a disabled button.

_{🤖 Generated by Anthropic Claude Code (Claude Opus 5.5); posted via @dawsontoth.}

🤖 Generated with Claude Code

Related PRs: #1846 overlaps (this PR's base; it edits the same table definition and test file), 3 others independent
Complexity: easy

_{Review-Coverage: authored=claude; ran=gemini,codex; adjudicated=domain; blocked=cursor-composer(no-receipt),cursor-muse(no-receipt); declined=cursor-grok,cursor-kimi; rounds=3; full=1 @ f07861f}

_{Review-Attention: read ~3m (decisions: wrapper-always-vs-only-when-disabled, focusable-wrapper-vs-aria-disabled, pending-state-tab-stop) @ f07861f}

[gemini-code-assist]

Code Review

This pull request wraps the "Remove Domain" button in a div container inside the TooltipTrigger to ensure the tooltip is displayed even when the button is disabled, and adds corresponding unit tests. The review feedback suggests improving accessibility for keyboard and screen-reader users by adding a conditional tabIndex to the wrapper div when the button is disabled.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	72.24%	11605 / 16063
🔵	Statements	72.35%	12367 / 17092
🔵	Functions	66.67%	3061 / 4591
🔵	Branches	67.72%	9066 / 13386

File Coverage

No changed files found.

Generated in workflow #2245 for commit d56bf21 by the Vitest Coverage Report Action