-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[charts-pro] Add chart title and caption to export demo #19524
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[charts-pro] Add chart title and caption to export demo #19524
Conversation
Deploy preview: https://deploy-preview-19524--material-ui-x.netlify.app/ Updated pages: Bundle size report
|
ec092cd
to
b4ab616
Compare
CodSpeed Performance ReportMerging #19524 will not alter performanceComparing Summary
Footnotes |
2e0429b
to
efd5d94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The feature looks good. I'm wondering if a better apporach for the user would be to use composition with different order.
<DataProvider>
<Toolbar />
<div ref={chartRef}>
<p>The Title</p>
<ChartSurface>{...}</ChartSurface>
<p>caption</p>
</div>
</DataProvider>
`.${legendClasses.root}`, | ||
) as HTMLElement | null; | ||
const stack = document.createElement('div'); | ||
const chart = document.body.firstElementChild!; | ||
|
||
if (legend) { | ||
legend.style.display = 'flex'; | ||
} | ||
stack.style.display = 'flex'; | ||
stack.style.flexDirection = 'column'; | ||
stack.style.width = 'fit-content'; | ||
stack.style.margin = 'auto'; | ||
|
||
document.body.appendChild(stack); | ||
|
||
const title = titleRef.current; | ||
if (title) { | ||
const titleClone = title.cloneNode(true) as HTMLSpanElement; | ||
stack.appendChild(titleClone); | ||
} | ||
|
||
document.body.removeChild(chart); | ||
stack.appendChild(chart); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might worth some comments like:
- Create a parent stack div to glue organize elements
- Move the title in the stack
- Move the chart as a next child
- Move the caption as a next child
So in the demo we should render the chart using composition instead of the normal way? |
Not really, because with composition you don't need to modify the What about adding a "Downloaded from mui.com" bellow the chart that appears only at export, and mofiy the sentence as follow Instead of "For example, you can add the title and caption to the exported chart, as shown below:"
|
That makes sense, but does it make sense to specify that in the We have a composition section for the export. What about updating that example with a title and a caption and mentioning in the |
Looks good 👍 |
Done 👍 |
Add a demo that shows how to export the chart title and caption. These aren't part of the chart per se, so they aren't exported by default. This demo is meant as a guide for users looking to customize their export.
Exported PNG: