Repository navigation
Conversation
Add a shared hover: 'capture' | 'passthrough' option to brushX and zoomX. With passthrough, idle hover over the zoom surface or the blank brush area resolves chart point focus and tooltips through the host's control containment check. Brush selections and handles, active gestures, clicks, wheel, and keyboard input stay with the control. The default keeps the current behavior.
…ocus A passthrough zoom surface needs focus for default wheel activation. Taking that focus no longer clears the resting pointer focus, so an accepted wheel zoom re-resolves the tooltip at the last pointer position. Cache the brush overlay lookup and cover focus returning after a brush drag ends.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 2 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 9 files. (10 skipped:… | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
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: allowing brushX and zoomX to pass idle hover to point focus. 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 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 9 files. (10 skipped: 10 unsupported.)
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Autopilot is currently an internal CodeRabbit preview.
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 @coderabbitai help to get the list of available commands.
🦋 Changeset detectedLatest commit: 92b48a9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 12 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Thanks Greg, I checked the unchanged runtime at 92b48a9 against current main in isolated Chromium with real mouse input. Default and explicit capture kept focus inside the control. Passthrough focused points, drag movement cleared point focus, and idle hover restored it. None of those gestures selected a chart point. Matched production fixtures measured these gzip increases, in decimal kB:
The cost summary needs a refresh. The current diff raises seven source ceilings, not two, and the zoom ceiling rises by about 0.094 kB. Those changes still need approval. The PR description also says F-307 while the actual entry is F-315, which overlaps other pending contributions. This is bounded browser and bundle verification, not a full repository validation pass. Touch, pen, wheel behavior and packed consumers still need verification before merge. |
|
Follow-up on 92b48a9: wheel zoom and accepted panning now pass the isolated Chromium checks too. Focus stays positioned correctly after wheel zoom, clears during a pan, and returns at the updated point position afterward. Accepted mouse pans and Chromium touch/pen input emulation all pass. I also checked default, capture and passthrough modes for both controls with mouse, touch and pen input. Brush callbacks fire, gestures do not select chart points, and idle pen hover follows the selected mode. Touch and pen gestures were separated beyond D3’s existing 500 ms ghost-click suppression window, so this does not claim immediate mixed-device switching or physical-device coverage. Full repository CI and the bundle-cost review are still outstanding. |
This branch has not been deployed
2 participants
With
brushXorzoomXon a plot, hovering that plot shows no point focus or tooltip. The controls cover the plot and capture every pointer event, and the host clears focus whenever the pointer is inside a control (renderer.ts). We hit this in a production app and had to move brush and zoom into a separate overview chart to keep point tooltips on the main plot.This adds one option shared by both controls:
hover?: 'capture' | 'passthrough'(ChartControlHover). The default,'capture', keeps today's behavior.With
'passthrough':It works through the existing control-containment check, via an internal
passesHover(target)on control instances. There is no CSS change, and the Canvas host shares the same renderer path.Verification:
capture,passthrough, drag suppression, focus returning after a drag, brush selection capture, and re-resolve after click then wheel.Bundle cost:
Not tested: real browsers and real touch or pen devices; everything above ran under jsdom.
package:checkfails locally at the React 18 packed-consumer step withERR_PNPM_PEER_DEP_ISSUES, andmainfails the same way here, so this looks environmental.Includes a minor changeset and API-FRICTION entry F-307. That ID will need renumbering if another friction entry lands first.
Summary by CodeRabbit
passthroughhover option. While the pointer is idle over eligible plot areas, nearby points can receive focus and display tooltips; the defaultcapturebehavior is unchanged.