[charts] Fix radar hover #17134
[charts] Fix radar hover #17134
Conversation
alexfauquette
added
breaking change
component: charts
|
Thanks for adding a type label to the PR! 👍 |
|
Deploy preview: https://deploy-preview-17134--material-ui-x.netlify.app/ |
CodSpeed Performance ReportMerging #17134 will not alter performanceComparing Summary
Benchmarks breakdown
|
packages/x-charts/src/internals/plugins/featurePlugins/useChartPolarAxis/useChartPolarAxis.ts
Outdated
Show resolved
Hide resolved
packages/x-charts/src/internals/plugins/featurePlugins/useChartPolarAxis/useChartPolarAxis.ts
Outdated
Show resolved
Hide resolved
packages/x-charts/src/internals/plugins/featurePlugins/useChartPolarAxis/useChartPolarAxis.ts
Outdated
Show resolved
Hide resolved
| export const generateSvg2rotation = | ||
| (center: { cx: number; cy: number }) => (x: number, y: number) => | ||
| Math.atan2(x - center.cx, center.cy - y); | ||
|
|
||
| export const generateSvg2polar = | ||
| (center: { cx: number; cy: number }) => | ||
| (x: number, y: number): [number, number] => { | ||
| const angle = Math.atan2(x - center.cx, center.cy - y); | ||
| return [Math.sqrt((x - center.cx) ** 2 + (center.cy - y) ** 2), angle]; | ||
| }; | ||
|
|
||
| export const generatePolar2svg = |
There was a problem hiding this comment.
Are we converting between cartesian to/from polar coordinates? Would it make sense to call these polarToCartesian or cartesianToPolar instead?
There was a problem hiding this comment.
I preferred svg over cartesian to avoid the confusion with the cartesian charts
| * For a pointer coordinate, this function returns the value and dataIndex associated. | ||
| * Returns `null` if the coordinate is outside of values. |
There was a problem hiding this comment.
It can also return undefined, right? Should we update the docs?
There was a problem hiding this comment.
It should not be possible. The computation of the dataIndex in the previouse selector is here to prevent that
There was a problem hiding this comment.
Since on line 43 we're doing axisConfig.data?.[dataIndex], if axisConfig.data is null or undefined, then this function will return undefined, won't it?
Same if dataIndex is outside the bounds of axisConfig.data.
Remaining:
Add test on radar chart for the axis highlight