[Fix] Disabled ticket actions explain themselves to assistive technology, Fixes #409

[juanmaguitar]

Why

The ticket card disables its actions while a link, switch, delete, trunk update, install or build runs, and used to do so silently: one shared gate, no sentence. The one button that did explain itself, Update this ticket to the current trunk, carried its reason as title, which Chromium does not show on a disabled control and assistive technology skips because the control has the real disabled attribute. A contributor saw a dead button with no way to learn why. #409 has the full case.

The dirty-trunk question had a second, smaller wrong: Discard them and start clean and Save them as a patch, then start clean… are false when the ticket already has parked work, because the switch restores that work and nothing starts clean.

What changes

• src/renderer/ticket-actions.cjs holds the reason for every branch of the shared gate (ticketActionDisabledReason), the two extra guards the move onto trunk adds because it rewrites the checked-out tree (rebaseDisabledReason, the dev server and a discard in flight), and the dirty-trunk question with both wordings (dirtyTrunkQuestion). Order follows discardDisabledReason: the action already underway first, then the process the contributor can wait for or stop.
• ReasonedButton in index.jsx is the house shape lifted out of DiscardChangesLink: Tooltip + accessibleWhenDisabled + description={reason} when there is a reason, the plain Button otherwise. DiscardChangesLink now builds on it. Every ticket action goes through it: Unlink, the branch rows (switch, delete work), the four dirty-trunk answers, Link ticket in both views, and the move onto trunk.
• Link ticket keeps a plain disabled for the empty field. That is not a blocked action, so it gets no sentence.
• When the ticket has parked work the answers read Save them as a patch, then continue on #N… and Discard them and continue on #N; the carry is not offered, as before.

Deliberately not in this PR: the button staying enabled after a rebase conflict (the third item in #409, needs state that outlives the refusal), the TextControl for the ticket number (a disabled text field has no tooltip shape in the component library), and src/main.js (untouched, another change owns it).

How to test this

Platform: either, macOS or Windows. Build: the Buildkite artifact for this branch at head 1fb75e1, or a local npm start from the repository root.

Starting state: a site with a linked ticket whose base predates trunk, so the card shows "Trunk has moved since this ticket started." A site cloned before trunk last moved, then Update to latest trunk on a second site, gets there; the journey below stages the same thing.

1. Click Start dev server (or Install dependencies). Expected: Update this ticket to the current trunk is greyed, still reachable with Tab, and hovering or focusing it shows "Stop the dev server before updating the ticket." (or "Wait for the installation to finish."). A screen reader reads the sentence as the button's description.
2. While the install runs, Tab to Unlink, Link ticket and, if the site has another ticket branch, switch and Delete this ticket's work. Expected: each is greyed and shows "Wait for the installation to finish."
3. Stop the server or let the install finish. Expected: every button is enabled again and the tooltip is gone.
4. Edit a file on trunk with no ticket linked, then link a ticket that already has work on this site. Expected: the yellow panel says "This ticket already has its own work here, so these edits cannot come along into it." and offers Save them as a patch, then continue on #N…, Discard them and continue on #N and Cancel. No "start clean" wording, no Take these edits into #N.
5. Same edit, link a new ticket. Expected: Take these edits into #N, Save them as a patch, then start clean…, Discard them and start clean, Cancel, unchanged from before.

What must not have happened: a button that is greyed with no tooltip and no description; a disabled button that still fires its action on click or Enter; the dirty-trunk panel losing an answer it offered before; the branch rows or the Unlink action disabled while the app is idle.

The layer-4 journey tests/e2e/journeys/ticket-rebase.spec.js clicks the move button by its accessible name on both platforms; run from the repository root with npx playwright test --project=journeys tests/e2e/journeys/ticket-rebase.spec.js. It covers the happy path, not the disabled state, so steps 1 to 3 are the manual pass. Unit coverage is tests/unit/ticket-actions.test.cjs (from the repository root: node --test tests/unit/ticket-actions.test.cjs). On the old code there is nothing to test: the gate was an inline boolean in index.jsx and the wording was inline JSX, which is the bug the test now pins.

Risks and limitations

• The self-review raised 2 🔵 [fix here] findings; both are fixed in 1fb75e1, none deferred. Everything below this line is what is deliberately not fixed here. See the review outcome for the full record.
• accessibleWhenDisabled keeps the button in the tab order and uses aria-disabled, so a Playwright click() on it waits for enabled as before, but a test that asserted the disabled attribute directly would need toBeDisabled(). No test in the repository does.
• The ticket number in the parked-work answers is read off the ref the switch was asked for, because only the new-ticket refusal in src/main.js carries ticket and files. That file is owned by another change, so the count stays absent on that path and the sentence reads "You have uncommitted changes on this site" rather than naming a number. Carrying both through the other refusal is the tidier fix and belongs wherever src/main.js next opens.
• The Link ticket field itself (TextControl) is still silently disabled by the gate; the button beside it explains.
• Unrelated finding from the same self-review that is not filed as an issue, recorded here: the ---/+++ header heuristic in rewritePatchPaths and splitPatchSections (src/patch-plan.cjs) is line-local, so a hunk that removes a line starting -- right before one adding a line starting ++ reads as a file header and the patch is refused with a nonsense reason. Never a bad write, Git decides. Tracking hunk state (the @@ counts) closes it.
• After a rebase conflict the move button stays enabled and a second click repeats the same refusal; harmless, left for a later change.

Related

Fixes #409. Follow-up to #408 (which moved the ticket panel's feedback under the controls).

---

Design decisions and alternatives considered

• One wrapper (ReasonedButton) rather than a second copy of the Tooltip block per button. DiscardChangesLink keeps its signature and callers; it is now three lines over the wrapper.
• The reason for the move onto trunk is computed separately from the shared gate instead of reusing discardDisabledReason, because that function's first three branches (patch loading, load failed, no changes) do not apply to a tree rewrite that is not a discard, and its sentences end in "before discarding changes".
• ticketActionsBlocked stays as Boolean(ticketActionsReason) so the two TextControls keep their gate without a second expression that could drift from the sentence.
• The dirty-trunk labels take canCarry from main's answer, as before; the module only words it. The "continue on #N" form is what the switch actually does, per Disabled ticket-card buttons explain themselves through title, which Chromium hides and assistive tech skips #409.

Review outcome (required — see AGENTS.md)

2 [fix here] · 3 [follow-up] — both [fix here] fixed in 1fb75e1.

Fixed:

• Architecture 🔵 — ReasonedButton returned <Button> without a reason and <Tooltip><Button></Tooltip> with one, so React saw a different element type at the same position and remounted the <button> on every flip of the gate. That discards what accessibleWhenDisabled buys: the keyboard user who just activated the control loses the focused element and focus falls back to the document, and the busy animation restarts on the buttons that pair isBusy with the same flag. The Tooltip is now always rendered; with no text it renders its anchor and no popover (verified in @wordpress/components 28.13.0, build/tooltip/index.js).
• Architecture 🔵 — rebaseDisabledReason checked discarding after the shared gate, so an install or build would be reported over a discard already rewriting the tree. discardDisabledReason puts it first, and so did this button's old inline title. It leads now, with the ordering pinned by the test.

Deferred, with reasons:

• Tests 🔵 [follow-up] — the canCarry: false question was asserted with a ticket number and a file count that the app could not produce, because only the new-ticket refusal in src/main.js attaches ticket and files. Rather than mark the wording unreachable, the renderer now derives the ticket from the ref it asked to switch to, so the #N form ships. The count still does not; carrying it through the other refusal needs src/main.js, which another change owns. Noted in Risks.
• Architecture 🔵 [follow-up] — the ticket-number TextControl beside "Link ticket" is still disabled with no reason. TextControl has no accessibleWhenDisabled, so it needs a different shape (a visible sentence in the row, or readOnly with aria-describedby) rather than this PR's wrapper. Noted in Risks.
• Tests 🔵 [follow-up] — none of the new tests could fail on the old code for the reported bug, because the bug was the wiring in index.jsx, which §1 of the review standard puts out of the suite's reach by construction. Taken as the repo's chosen trade-off; the module header now names ReasonedButton as the only sanctioned way to disable a ticket action, so a future review has something to point at.

Checked and clear in the review: isBusy with accessibleWhenDisabled (the no-op click handlers are spread after the caller's props, so a gated click cannot reach saveTicket); Tooltip inside the text span and the flex row (portalled popover, VisuallyHidden description, no layout or focus effect); the dirty-trunk question sentence byte-identical for both canCarry values; Button forwarding description and accessibleWhenDisabled on the installed 28.13.0; and the ticket-rebase journey's locator and auto-wait, since description does not enter the accessible name and Playwright's enabled check honours aria-disabled. Security, performance and cross-platform: nothing to report, a renderer string module with no IPC, spawn, path or persistence surface.

• Review: not run — CodeRabbit; its check reads pass but its message reads "Review rate limited", and an @coderabbitai review after the window it named was refused the same way. No CodeRabbit finding exists for this PR, at any severity.
• Review: completed — fresh agent context, no involvement in writing the change, against .github/instructions/code-review.instructions.md; reviewed head 909258d / base f31e5df; npm run lint clean and npm test 1289 pass / 0 fail at that revision.
• Since review: 909258d → 1fb75e1, the two [fix here] findings plus the ticket-number fallback; lint clean, npm test 1289 pass / 0 fail, npm run build:once clean. Not re-reviewed in a fresh context.

Screenshots or recording

No screenshot. The visible change is a tooltip and a screen-reader description on buttons that were already greyed; the state that shows it needs a running install or dev server, which the manual pass above stages. The dirty-trunk panel's new labels are quoted in step 4.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KJJSFQMq1ahmubgNNxJwqz

[coderabbitai]

Warning

Review limit reached

Next included review available in 55 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: d989a2bc-f236-4d90-bdd3-7fdf7c4cd772

📥 Commits

Reviewing files that changed from the base of the PR and between d7319be and 19502d7.

📒 Files selected for processing (3)

• src/renderer/index.jsx
• src/renderer/ticket-actions.cjs
• tests/unit/ticket-actions.test.cjs

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[juanmaguitar]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.