[charts] Extract bar and line plot logic into reusable hooks #18541
Conversation
a6a86e9 to
cb09f26
Compare
|
Deploy preview: https://deploy-preview-18541--material-ui-x.netlify.app/ Bundle size reportTotal Size Change: 🔺+714B(+0.01%) - Total Gzip Change: ▼-16B(0.00%) Show details for 100 more bundles (22 more not shown)@mui/x-charts/SparkLineChart parsed: 🔺+232B(+0.13%) gzip: 🔺+74B(+0.13%) |
CodSpeed Performance ReportMerging #18541 will not alter performanceComparing Summary
|
| export function useAreaPlotData( | ||
| xAxes: ComputedAxisConfig<ChartsXAxisProps>, |
There was a problem hiding this comment.
What about adding the drawingArea as a first arg for coherence with the bar plot.
Could be useful if at some point we need to add algorithm to simplify line shape
There was a problem hiding this comment.
I could add it, but it wouldn't be used for anything. Would it make sense to add it later when we need it?
What benefit do you see? We'd need to be more careful making changes to them in the future, which wouldn't be ideal. |
I don't see why we would export it right now too |
| /** | ||
| * Solution of the equations | ||
| * W = barWidth * N + offset * (N-1) | ||
| * offset / (offset + barWidth) = r | ||
| * @param bandWidth The width available to place bars. | ||
| * @param numberOfGroups The number of bars to place in that space. | ||
| * @param gapRatio The ratio of the gap between bars over the bar width. | ||
| * @returns The bar width and the offset between bars. | ||
| */ | ||
| function getBandSize({ | ||
| bandWidth: W, | ||
| numberOfGroups: N, | ||
| gapRatio: r, | ||
| }: { | ||
| bandWidth: number; | ||
| numberOfGroups: number; | ||
| gapRatio: number; | ||
| }) { | ||
| if (r === 0) { | ||
| return { | ||
| barWidth: W / N, | ||
| offset: 0, | ||
| }; | ||
| } | ||
| const barWidth = W / (N + (N - 1) * r); | ||
| const offset = r * barWidth; | ||
| return { | ||
| barWidth, | ||
| offset, | ||
| }; | ||
| } |
There was a problem hiding this comment.
nitpick, I generally prefer the non-main function of a file to be above, or on a new file 😆
makes it easier to find
|
#12975 I was thinking about issues like this one where the berElement slot is not the correct place to do the customisation, and having the data preprocessed to do a custom rendering would be easier. But it's an open question. Since you're working on creating a new bar plot for the preview, you will get insight about what are the difficulties |
Yeah, I see the point. I think we should work towards that, but I'm not comfortable exporting these hooks now as they haven't been thought through and as soon as we export them, we need them to be stable 😅 |
Part of #15383. Cherry-picked from #18267.
Extract plot logic into separate files for reuse. In #18267, these hooks will also be used by the preview components.