[charts] Allow the creation of custom HTML components using charts data #15511
Conversation
JCQuintas
added
enhancement
|
Deploy preview: https://deploy-preview-15511--material-ui-x.netlify.app/ Updated pages: |
CodSpeed Performance ReportMerging #15511 will not alter performanceComparing Summary
|
|
@alexfauquette check this out. It seems to be possible to fully remove the wrapper Still need to fix the tooltips though 😆 |
This reverts commit 468c05e.
Why are you trying to remove it? I'm looking at the Recharts DOM structure and it seems quite similar our current structure <div class="recharts-responsive-container"> // Takes full parent size
<div class="recharts-wrapper"> // Full size and position relative
<svg class="recharts-surface">...</svg> // Full size
<div class="recharts-legend-wrapper">...</div> // Position absolute
</div>
</div>Do you have a particular use case in mind where it's needed? |
Not exactly needed, but feels useless. My goal is that the user will be able to do: function MyComponent() {
const a = useBarSeries();
return (
<div>
{a?.seriesOrder.map((id) => {
const series = a.series[id];
return <div key={id}>{series.data.join('')}</div>;
})}
</div>
);
}
const MyChart = () => {
return (
<div style={{ display: 'flex', flexDirection: 'column' }}>
<ChartDataProvider ... >
<ChartsSurface ...>{...}</ChartsSurface>
<MyComponent />
</ChartDataProvider>
</div>
);
});And this will result in a very simple initial structure. <div> // provided by user
<svg /> // chart
<div /> // provided by user
</div>
// current
<div> // provided by user
<div> // responsive container
<svg /> // chart
</div> // provided by user
<div />
</div>There is only one layer the user needs to think about for classes and props. |
|
I was surprised that this work <ChartDataProvider ... >
<ChartsSurface ...>{...}</ChartsSurface>
<MyComponent />
</ChartDataProvider>But it's because I thaugh we measure the parent size but we are measuring the element itself I'm ok with the idea. I would be interested to see a component. For example the BarChart with an HTML legend |
Would something like this be good? https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-qvc5q3?file=%2Fsrc%2Fdemo.tsx%3A33%2C31 Is there anything specific you would like to see? 😆 |
Yes, it looks nice. I hope people will not try too crazy stuff with the div that wrapp the SVG and the legend together. |
What do you mean by "too crazy"? |
|
Arbitrary absolute positioning also works: https://codesandbox.io/p/sandbox/mui-mui-x-x-data-grid-forked-xy38kf?file=%2Fsrc%2Fdemo.tsx%3A22%2C44&workspaceId=836800c1-9711-491c-a89b-3ac24cbd8cd8 |
It's just that devs will probably use that with all the display options possible. But that's not a real issue. With the two examples you made it should be ok 👍 |
|
Regarding docs any idea where it should stand? I suppose
Or inside composition we have a section for |
|
created the PR: #15545 |
What about a third option that would be to start with a general introduction about And then a subsection explaining that we provide a helpêr |
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
github-actions
bot
removed
the
PR: out-of-date
| const element = svgRef.current; | ||
| if (element === null) { | ||
| return 'no-point-found'; | ||
| } | ||
|
|
There was a problem hiding this comment.
Yes, that's why the check for element === null was initially at the beginning. If the element is null, we will not attach events to it so we can do early return.
If your concern is about the risk of mutation, notice it's the SVG ref. The moment it's null, it means we have no chart to display, so we have bigger issue in that case :)
Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com> Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
JCQuintas
changed the title
divSigned-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
| React.useEffect(() => { | ||
| if (svgRef.current === null) { | ||
| return undefined; | ||
| } | ||
| const element = svgRef.current; | ||
| if (element === null) { | ||
| return () => {}; | ||
| } |
There was a problem hiding this comment.
If you don't do this revert, You should be able to remove the ! of element. I don't know why TS does not take into account the early return, because event mutation is not an explanation
const x = { current: document };
const a = x.current;
x.current = null;
console.log(x) // { current: null }
console.log(a) // document
…ta (mui#15511) Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com> Signed-off-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com> Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Resolves #13852
Changelog
divwrapping them anymore. All props are now passed to the rootsvginstead of thediv.ChartDataProviderandChartsSurfaceare now fully divided — Learn more.