fix(devtools): claim the inspect click in the capture phase - #516
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe source inspector now handles armed clicks during the document capture phase. Tests cover propagation-stopping ancestors and ordinary clicks without the inspection hotkey. A patch changeset documents the update. ChangesSource inspection click handling
Merge Risk: ⚪ Minimal · up to The change prevents inspect clicks from triggering underlying button actions or being swallowed by stopping ancestors, while preserving ordinary clicks. No actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant User
participant document
participant SourceInspector
participant Ancestor
User->>document: Click an inspected element
document->>SourceInspector: Invoke onInspectClick during capture
SourceInspector->>SourceInspector: Open or copy the source
document->>Ancestor: Continue dispatch for an unarmed click
Ancestor-->>User: Handle the ordinary click
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
The source inspector listened for clicks in the bubble phase, so by the time it ran the page had already acted on the click: React had dispatched its synthetic onClick, a router link had navigated. preventDefault() cancels only the browser's own default action, so none of that could be undone. Worse, any ancestor calling stopPropagation() -- a modal or dropdown that closes on an outside click -- stopped the event before it reached document, so inspecting inside one silently did nothing at all. Claiming the click in the capture phase fixes both. It costs the page nothing: the handler returns immediately unless the inspect hotkey is held over an element carrying data-tsd-source, and it already performs the open-in-editor or copy itself.
de3c8dd to
153a720
Compare
🎯 Changes
Holding the inspect hotkey and clicking an element:
onClickSourceInspectorlistened ondocumentin the bubble phase. By then React has already dispatched its syntheticonClickand a router link has already navigated, andpreventDefault()cannot undo either — it only cancels the browser's own default action. The second row is the sharper one: an ancestor callingstopPropagation(), which is every overlay that closes on an outside click, stops the event before it reachesdocument, so the handler never ran and an inspect click inside a modal silently did nothing.{ capture: true }fixes both, and costs the page nothing: the handler returns immediately unless the hotkey is held over an element carryingdata-tsd-source, and it already performs the open-in-editor or copy itself rather than relying on anything downstream.Still open, and out of scope here:
mousedown/pointerdownare untouched, so a drag or focus wired to those still starts on an inspect click.Verification —
examples/react/basic, with a throwaway component holding a plain<button onClick>and a button inside a<div>whose native click listener callsstopPropagation(). Onmain, an inspect click on the plain button copied…/inspect-click-repro.tsx:35:7and took its counter 1 → 2; the same click inside the modal took that counter 1 → 2 and copied nothing. With this change both copied their path and neither counter moved, while ordinary clicks without the hotkey still increment both.source-inspector.test.tsxpins the modal case; it fails onmain.✅ Checklist
pnpm test:pr.🚀 Release Impact
Summary by CodeRabbit