fix: compute correct dataX/dataY in events #186
Conversation
✅ Deploy Preview for svelteplot ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
📦 Preview package for this PR is published! Version: Install it with: npm install svelteplot@pr-186
# or install the specific version
npm install svelteplot@0.4.2-pr-186.0 |
|
📦 Preview package for this PR is published! Version: Install it with: npm install svelteplot@pr-186
# or install the specific version
npm install svelteplot@0.4.2-pr-186.1 |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes event coordinate calculations for pointer events by switching from layerX/layerY to clientX/clientY and improving scale inversion logic for band scales. The changes ensure more accurate data mapping from pointer events, particularly in faceted plots.
Key changes:
- Enhanced event coordinate calculation using client coordinates and proper facet container detection
- Improved band scale inversion to handle positioning within the scale range correctly
- Refactored import statements to use more specific type imports
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/lib/marks/helpers/events.ts | Enhanced event coordinate calculation and band scale inversion logic |
| src/lib/marks/helpers/RectPath.svelte | Refactored imports to use more specific type imports |
| origEvent.clientX - facetRect.left + (options.marginLeft ?? 0); | ||
| const relativeY = origEvent.clientY - facetRect.top + (options.marginTop ?? 0); |
There was a problem hiding this comment.
The margin calculations appear incorrect. When converting from client coordinates to plot coordinates, margins should be subtracted, not added. This would place events at incorrect positions within the plot area.
| const extent = range[1] - range[0]; | ||
| const posInRange = (position - range[0]) * Math.sign(extent); | ||
| const index = Math.floor(posInRange / eachBand); | ||
| return domain[index]; |
There was a problem hiding this comment.
Array access without bounds checking could cause runtime errors. If the calculated index is negative or exceeds the domain length, this will return undefined unexpectedly. Add bounds checking: return index >= 0 && index < domain.length ? domain[index] : undefined;
|
📦 Preview package for this PR is published! Version: Install it with: npm install svelteplot@pr-186
# or install the specific version
npm install svelteplot@0.4.2-pr-186.2 |
|
📦 Preview package for this PR is published! Version: Install it with: npm install svelteplot@pr-186
# or install the specific version
npm install svelteplot@0.4.2-pr-186.3 |
resolves #183
Event handling improvements:
events.ts, improved the calculation of event coordinates by:clientX/clientYinstead oflayerX/layerYfor more accurate positioning.Scale inversion logic:
invertScalefunction for band scales to compute the correct domain value based on the event's position within the plot, leading to more accurate data mapping from pointer events.