[docs] Refine charts overview page #17447
Conversation
|
Thanks for adding a type label to the PR! 👍 |
|
Deploy preview: https://deploy-preview-17447--material-ui-x.netlify.app/ Updated pages: Bundle size reportTotal Size Change: 0B(0.00%) - Total Gzip Change: 🔺+61B(0.00%) Show details for 100 more bundles (22 more not shown)@mui/x-charts parsed: 0B(0.00%) gzip: 0B(0.00%) |
CodSpeed Performance ReportMerging #17447 will not alter performanceComparing Summary
|
|
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
docs/src/modules/components/overview/charts/mainDemo/HeatmapDemo.tsx
Outdated
Show resolved
Hide resolved
docs/src/modules/components/overview/charts/mainDemo/BarChartDemo.tsx
Outdated
Show resolved
Hide resolved
docs/src/modules/components/overview/charts/featuresHighlight/FeaturesHighlight.tsx
Outdated
Show resolved
Hide resolved
docs/src/modules/components/overview/charts/mainDemo/DownloadDemo.tsx
Outdated
Show resolved
Hide resolved
docs/src/modules/components/overview/charts/advancedCharts/AdvancedChartDemo.tsx
Outdated
Show resolved
Hide resolved
docs/src/modules/components/overview/charts/essentialCharts/ScatterChartDemo.tsx
Show resolved
Hide resolved
|
Sorry, just noticed this, but shouldn't we align the left sections with the titles to the top? |
|
I removed the code example since they bring less value than they create visual disturbance |
|
@alexfauquette should we merge this? No idea why the perf decreased though 😆 |
|
@alexfauquette I pushed some small styling adjustments and leaving some more generic feedback here:
|
| // React.useEffect(() => { | ||
| // if (active) { | ||
| // return undefined; | ||
| // } | ||
| // const timeout = setTimeout(() => setSelected((p) => (p + 1) % 4), 5000); | ||
|
|
||
| return () => clearTimeout(timeout); | ||
| }, [selected, active]); | ||
| // return () => clearTimeout(timeout); | ||
| // }, [selected, active]); |
There was a problem hiding this comment.
@noraleonte was this comment only for debugging purpose?
There was a problem hiding this comment.
Yeah, just noticed it and fixed 😅 Sorry
Yes, the main issue was the highlight demo that then get two hovers effect. A first one when entering the cell, and a second one when moving on items. Was not clear what is part of the library vs what is fancy UI
Ok, I will fix that :)
I'm more inclined of having a selected number of feature that are visible out of the box. For en exhaustive showcase of the available charts, e will create a dedicated page like this one The issue with the other display is you need to click on small buttons to see the demo. I want us to identify the most important feature and show them. For example, a company was concerned we do not support mixing libe, bars, and others. Now it should be obvious thanks to the "Advanced Features: Multi axes and series" demo. And new commer just need to scroll to see it.
Good point. I will do it
I thought one day I will find something more interesting to display 🙃 But yeah lets go full on pockemon starter pack 🚀
I could subsample the data points to avoid those massive pick which correspond to monthly download (probably dependabot) |
|
For some devices (Surface Pro 7) the pie and radar charts look off. While these devices are rare, should we still fix it? |
docs/src/modules/components/overview/charts/essentialCharts/PieChartDemo.tsx
Outdated
Show resolved
Hide resolved
…ui-x into refine-charts-overview-page
|
The heatmap hover seems a bit weird because it's losing the border radius. Should we keep it when hovering? Screen.Recording.2025-07-02.at.15.48.27.movI didn't realize these were clickable. Should we make it clearer? E.g., color change on hover, and maybe show the active state with a different color? Screen.Recording.2025-07-02.at.15.49.16.movThe space between the zoom buttons and the export button seems too big. We used to have a divider, but if we don't want it should we apply This section seems a bit weird to me: The stacking section already has highlighting. Also, the toggle group makes me feel like highlighting is the opposite of stacking, which it isn't. Should we remove the toggle group altogether since the stacking section already has highlighting? The border radius of the button should be 4px smaller than the toggle group because the padding is 4px. Currently: Button with 8px border-radius: The scatter chart example (and the others in this section) are missing the translucent tooltip of the other examples: The pie chart example is actually a donut chart. While it's clear to us that a donut chart is a specific case of the more general pie chart, I'm not sure if it's so clear to other people. Not a big deal, just wanted to make sure we're aware that it might be confusing. |
docs/src/modules/components/overview/charts/advancedFeatures/ZoomAndPanDemo.tsx
Outdated
Show resolved
Hide resolved
docs/src/modules/components/overview/charts/advancedFeatures/ExportDemo.tsx
Outdated
Show resolved
Hide resolved
| const currencyFormatter = new Intl.NumberFormat('en-US', { style: 'currency', currency: 'USD' }) | ||
| .format; | ||
|
|
||
| function Export() { |
There was a problem hiding this comment.
Exporting isn't working well in dark mode:
Screen.Recording.2025-07-02.at.17.03.32.mov
This is kind of a bug.
In the docs example, the chart is exported with a white background. However, in the overview page the white background doesn't work well.
I can see the case for both options: sometimes users want the chart to be on a white background (e.g., if they're exporting it to send in an email), sometimes they might want the background to mimic the one on the page.
To solve this, I'm thinking that it might make sense to provide access to the print window before print so that users can customize what they want. For example, in this case, we'd just need to add data-mui-color-scheme="dark" to the html tag and CSS variables would recompute, causing the export to have a black background. However, this data-mui-color-scheme attribute is very specific to our use case, so providing a onBeforePrint: (iframe: HTMLIframeElement) might be useful.
What do you think?
There was a problem hiding this comment.
onBeforeExport would probably be more useful. We can then do that for every export type.
The print's defaults should be data-mui-color-scheme="light" by default imo though, to always for light-mode unless overridden.
There was a problem hiding this comment.
onBeforeExportwould probably be more useful. We can then do that for every export type.
Good point 👍
The print's defaults should be
data-mui-color-scheme="light"by default imo though, to always for light-mode unless overridden.
By default we don't add anything, so that will depend on the website's default styles. Users will be able to customize that with onBeforeExport.
There was a problem hiding this comment.
My point is that the print window should have a white background by default, so the light version of whatever is being printed should be used.
If we don't do it ourselves, then we should have a section in the docs telling devs that and how to change the behaviour.
There was a problem hiding this comment.
My point is that the print window should have a white background by default, so the light version of whatever is being printed should be used.
The print window has the default, I haven't changed that.
I suppose that if the page of the chart you're printing has a rule @media print { html { background: black; } }, then it's expected that the print window will have a black background.
I agree we can have an option (like the Data Grid has) to skip copying the styles from the original window to the print window so that print rules like those aren't copied over (and for other use cases as well).
There was a problem hiding this comment.
I agree we can have an option (like the Data Grid has) to skip copying the styles from the original window to the print window so that print rules like those aren't copied over (and for other use cases as well).
Than can be an option as well.
I suppose that if the page of the chart you're printing has a rule @media print { html { background: black; } }, then it's expected that the print window will have a black background.
I hardly doubt someone would do that intentionally nowadays though. 😆 At least they would probably be very intentional when doing it, so they would look how to do it for charts as well
Preview: https://deploy-preview-17447--material-ui-x.netlify.app/x/react-charts/