Repository navigation
fix: keep a click landing as a tooltip arms on its button - #1256
Conversation
A tooltip mounts once a pointer enters its trigger, mounting the trigger again. React renders that later than the pointer entering, so a click following it at once could press the old trigger and be released on the new one, and the browser fired no click: in WebKit, a click on the commit's delete button right after the pointer reached it was lost about one time in five. The tooltip now arms at once, before the press. Not unit-tested: the unit tests have no DOM. Checked with Playwright in WebKit, clicking the delete button as the pointer reaches it: 3 lost clicks in 15 before, none after; tabbing onto the composer's buttons still shows their tooltips.
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Synchronous tooltip arming resources/js/components/ui/tooltip.tsx |
arm now wraps the armed-state update in flushSync. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix
Possibly related PRs
- dev-null-sh/dev-null.sh#1150: This PR introduced lazy tooltip mounting and the arming state that this change updates.
- dev-null-sh/dev-null.sh#1247: This PR adds keyboard shortcuts that display through
ActionTooltip, which uses the changed tooltip behavior. - dev-null-sh/dev-null.sh#597: This PR adds keyboard actions that expose shortcuts through
ActionTooltip.
Suggested reviewers: xheaven
Merge Risk: 
cbd6f
-
The synchronous tooltip arming change has no established merge-blocking risk.
Pre-merge checks: 
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: preserving clicks when a tooltip arms synchronously on its button. Docstring Coverage Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.
-
Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Comment @coderabbitai help to get the list of available commands.
Summary
Since tooltips mount on first hover (#1150), a click that follows the pointer onto a button right away can get lost: the tooltip mounts the button again between the press and the release, so the browser fires no click. In WebKit, clicking a commit's delete button right after hovering it lost 15 clicks out of 20.
Changes
flushSync) when the pointer enters or the keyboard focuses its trigger, so the button is mounted again before the press lands on it. React otherwise renders after a pointerenter later, as a continuous event.Validation
production, none on this branch.ActionTooltipacts at once; deleting a commit confirms after one click.tsc, lint, format, node tests (206) and Vitest (722) pass locally. No PHP change.productiontoo (aResizeObserver loopwarning, WebKit'shistory.replaceStaterate limit).Independent review
Separate Claude agent, base
fa8d5407, heade580215f(same tree ascbd6fb81, only the commit footer removed since): CODE: PASS, VISUAL: N/A (only when the tooltip renders changes, not what it shows). Its one suggestion, tabbing onto a tooltip button inside a modal's focus trap, is checked above.Limitations
Summary by CodeRabbit