[charts-pro] Add zoom to heatmap #20708
Conversation
bernardobelchior
added
type: new feature
bernardobelchior
force-pushed
the
heatmap-zoom
branch
from
fd37907 to
8a1aa0c
Compare
|
Deploy preview: https://deploy-preview-20708--material-ui-x.netlify.app/ Updated pages: Bundle size report
|
CodSpeed Performance ReportMerging #20708 will not alter performanceComparing Summary
Footnotes |
bernardobelchior
force-pushed
the
heatmap-zoom
branch
2 times, most recently
from
7b39ace to
70586a2
Compare
bernardobelchior
force-pushed
the
heatmap-zoom
branch
from
70586a2 to
d118441
Compare
bernardobelchior
requested review from
a team and
prakhargupta1
as code owners
|
|
||
| const seriesData: HeatmapValueType[] = []; | ||
|
|
||
| let max = 0; |
There was a problem hiding this comment.
Nitpick
| let max = 0; | |
| let maxContribution = 0; |
| function HeatmapCell({ | ||
| ownerState, | ||
| x, | ||
| y, | ||
| width, | ||
| height, | ||
| ...props | ||
| }: HeatmapCellProps) { | ||
| return ( | ||
| <rect | ||
| x={x + 1} | ||
| y={y + 1} | ||
| width={width - 2} | ||
| height={height - 2} | ||
| {...props} | ||
| rx={4} | ||
| ry={4} | ||
| fill={ownerState.color} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
I assume this is used to improve performances by removing highlight hooks
There was a problem hiding this comment.
No, it was just so we could have rounded corners and some padding 😅
| useChartCartesianAxis, | ||
| useChartHighlight, | ||
| useChartProExport, | ||
| useChartBrush, |
There was a problem hiding this comment.
Are the brush necessary?
If yes, we should also add the <ChartsBrushOverlay />
There was a problem hiding this comment.
Yes, it's necessary because the zoom plugin has a dependency on it. I can add checks and make it optional, but wouldn't it be better to add the brush overlay instead?
3 participants
Fixes #20622.
Screen.Recording.2025-12-19.at.15.11.16.mov