Fold never-dismissing toasts into a corner pill so they stop swallowing clicks - #3198
Merged
Conversation
…ing clicks The toast stack is fixed, z-[9999] and pointer-events-auto, so a toast that never dismisses is a permanent click sink over the page. On this install the "Install out of sync" notice (duration: Infinity) sat on top of the CoS task form and silently absorbed every click aimed at Screenshot and Attach — no file picker, no error, nothing in the console, which is why it read as a broken button rather than something covering one. Measured in headless Chrome against the running instance at 1280x720: the notice blocks 520x86px starting at x=744, and the Attach label occupies x=744-837 — a direct hit. Removing the toast from the DOM made the same click open the picker. A duration: Infinity toast now collapses to a 40x40px corner pill after 8s (1600px^2, clear of the form); clicking the pill restores the full message, and it stays open while hovered or while focus is inside it. Collapsing hides the body with an inline display:none rather than unmounting it, so render-prop toasts that own their own dismiss timer (the agent-feedback card) keep them. Only unbounded toasts collapse. A finite duration is a bound the caller already chose and clears itself — folding one early would hide an action meant to stay offered for its whole life, e.g. the manuscript Undo-fix toast, whose 10s Undo button is the only undo there is once the source card unmounts. The pill takes its accessible name from string content; JSX and render-prop content have no text to take, so those call sites now pass `label`, and a11yConventions.test.js gains a fourth rule that fails the suite if a new one forgets — the same shape as the existing file-input guards, because nothing at runtime complains.
…ound, not the 8s default The card passes `duration: Infinity` only because it runs its own AUTO_DISMISS_MS (15s) dismiss timer, so the blanket 8s collapse hid its rating buttons for the last 7s of a lifetime the caller had in fact bounded. Re-opening the pill made it worse: that does not restart the card's inner timer, so a card re-expanded at 10s disappeared 5s later, potentially mid-click. Toasts whose bound lives in their content now pass `collapseAfter` to fold on their own schedule. It is a delay, not an opt-out — the value is validated so a non-finite one falls back to the default instead of switching the fold off, because this card clears its dismiss timer once expanded and an unbounded 480px card in the corner is the widest click sink of the set.
…ndently Both holds shared one `held` flag, so whichever ended last cleared the other's. Tab to a button inside a persistent toast, then sweep the pointer over the toast and off again: `mouseleave` released a hold that focus still owned, and 8s later the collapse applied `display: none` to the focused button and dropped focus to <body> — precisely the outcome the hold exists to prevent. The mirror case (focus leaving while the pointer stays) folded the toast out from under the cursor. Tracked as two booleans OR'd together. Both interleavings now have regression tests; both fail against the single-flag version.
…nded by keyboard
Activating the pill unmounts it, so a keyboard user pressing Enter on it
lost focus to <body> — the toast's own Reconcile/Reload/Ignore buttons
left the tab sequence and the next Tab restarted from the top of the
document, past everything on the page.
The revealed body now takes focus (tabIndex={-1}, programmatic only, so
it never sits between the page's own controls). Guarded on the pill
having actually held focus: a pointer activation in an engine that does
not focus buttons on click has nothing to rescue, and grabbing focus
anyway would take the focus hold and pin the toast open until the user
clicked elsewhere — the parked overlay this set out to remove.
…Element The focus-handover guard from the previous commit asked whether the pill was document.activeElement to decide "was this a keyboard activation?". Chrome focuses a <button> on mouse-down, so that reads true for an ordinary click: focus moved into the toast body, the body's onFocus took the focus hold, and nothing ever released it. Measured via CDP against the running page — after a real mouse click the toast was still expanded 11s later, with activeElement parked on the toast. That is the click-eating overlay this branch exists to remove, handed back to precisely the mouse users who reported it. detail === 0 is the actual signal: a click synthesized from Enter/Space carries no click count, a pointer click carries at least 1. The unit test could not have caught this. jsdom's fireEvent.click neither focuses the button nor sets detail, so an activeElement guard passes there and fails in the browser. Both tests now state their gesture explicitly (detail 0 for keyboard, 1 for pointer) and the pointer test models Chrome by focusing the pill first, so it fails against the old guard. Re-verified in headless Chrome: Enter hands focus to the toast body; a mouse click leaves focus alone and the toast folds itself away again.
… an apostrophe `balancedCallAt` treats `'` as a string opener, so JSX text like `<div>You're out of sync</div>` opened a "string" that never closed. The scan ran past the call's closing paren, returned null, and the caller skipped it silently — meaning the new `duration: Infinity` label rule missed exactly the shape it exists to catch: JSX content, which has no text for the collapsed pill to name itself with. Falling off the end now retries counting parens only. A well-formed string still returns on the first (string-skipping) pass, so a `)` inside a literal can't close the call early.
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
1 participant
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.
Summary
The Screenshot and Attach buttons on the CoS task form did nothing when clicked from a desktop browser — no file picker, no error, nothing in the console — while the same page worked from a phone. They were not broken. A notification was sitting on top of them.
The toast stack renders
fixed,z-[9999],pointer-events-auto. A toast created withduration: Infinity("Install out of sync", "Update available", "New build available") therefore parks in the bottom-right corner forever and absorbs every click aimed at whatever is underneath it. This install reportsoutOfSync, so that notice is present on every page load.Measured in headless Chrome against the running instance at 1280x720:
A direct hit, starting at the button's exact left edge. Deleting the toast node from the DOM made the identical synthetic click open the picker (
fileChooserOpened=0→1). The picker markup itself was fine in every engine and viewport tested.What changed
A
duration: Infinitytoast now folds into a 40x40px corner pill after 8s — 1600px², clear of the form. Clicking the pill restores the full message; it stays open while hovered or while focus is inside it. Toasts with a finite lifetime are untouched.display: nonetakes the body out of hit-testing and the a11y tree while leaving it mounted, so render-prop toasts that run their own dismiss timer keep them.collapseAfter.useAgentFeedbackToastsetsInfinityonly because its card runs a 15s dismiss; the 8s default would have hidden the rating buttons for the last 7s, and re-opening the pill does not restart that inner timer. It is a delay, never an opt-out — a non-finite value falls back to the default so a strayInfinitycannot switch folding off and hand back the overlay.mouseleaverelease a hold that focus still owned, and 8s laterdisplay: nonelanded on the focused button.detail === 0.document.activeElementis not a usable substitute: Chrome focuses a button on mouse-down, so it reads true for an ordinary click — which moved focus into the body, took the focus hold, and parked the toast open forever.a11yConventions.test.jsgains a fourth rule: aduration: Infinitytoast with JSX or render-prop content must passlabel, or its pill's entire accessible name is "Show notification". Nothing at runtime complains, so a guard is the only thing that catches it.Test plan
cd client && npx vitest run— 476 files / 5321 tests pass. 12 new tests cover collapse, the transient-toast exemption, re-expand, the two independent holds, both focus-handover gestures,collapseAfter, the non-finite rejection, and hide-not-unmount.cd client && npm run lint— clean.labelfromstaleBuildToast.jsxfails the new rule; reverting the slicer fallback fails the apostrophe regression test.Reviewers
codex[gpt-5.6-terra](capped 3/3) raised three real defects, all in the new collapse mechanism rather than the original fix — the self-managed 15s lifetime, the shared hold flag, and keyboard focus loss. Its remedy for the first (an opt-out for component-managed toasts) was rejected: that card expands to 480px and clears its own timer when open, making it the widest genuinely-unbounded click sink of the set. Its fix for the third shipped a browser-only regression (theactiveElementguard above) that its jsdom test could not see; caught by re-checking in Chrome.ollama[Qwythos-9B](capped, optional) rejected two false positives and surfaced one real hole: an apostrophe in JSX text opened an unterminated string in the new guard's slicer, sotoast(<p>You're out of sync</p>, { duration: Infinity })was skipped silently — the exact shape the rule exists to catch.