[charts] Add an overlay for "no data" or "loading" states #12817
Conversation
alexfauquette
added
the
component: charts
|
Deploy preview: https://deploy-preview-12817--material-ui-x.netlify.app/ Updated pages: |
|
|
||
| return ( | ||
| <StyledText x={left + width / 2} y={top + height / 2} {...props}> | ||
| Loading data ... |
There was a problem hiding this comment.
I guess you don't want to introduce a localization system for this text 😆
But people should be able to pass slotProps={{ loadingOverlay: { children: 'Chargement des données' } }} and right now I don't think it works
The following might do the trick:
| Loading data ... | |
| props.children ?? 'Loading data ...' |
There was a problem hiding this comment.
Sorry I didn't see it was in draft 👼
There was a problem hiding this comment.
No issue I was motivated to introduce a localization system, but the notion of props might be better as long as we do not see other translation needs
There was a problem hiding this comment.
as long as we do not see other translation needs
I agree with your approach
If this will likely be the only text in the foreseeable future, a prop is fine
If we think of some other texts that might be introduced soon, the localization system is required
|
Now that we have this feature, would it be feasible to also support animation images instead of the simple text for the loading and no data states? Animations could be supported through GIFs, animated SVGs or even Lottie files. The exact format supported should of course depend on the ease of implementation. I am suggesting this because such animations are briefly illustrated in the official MD2 docs, in the last paragraph of the Behaviour section of the Data Visualisation page (just above the Dashboards section). Should this be discussed in a separate issue? |
|
|
||
| return ( | ||
| <StyledText x={left + width / 2} y={top + height / 2} {...other}> | ||
| {message ?? 'Loading data ...'} |
There was a problem hiding this comment.
| {message ?? 'Loading data ...'} | |
| {message ?? 'Loading data…'} |
Side note, I think we need this string to be a prop for internationalization.
I would also expect the need for a localization prop down the line, considering e.g. Highcharts
| /** | ||
| * If `true`, a loading overlay is displayed. | ||
| */ | ||
| loading: PropTypes.bool, |
There was a problem hiding this comment.
We are missing the default value for the prop: https://deploy-preview-12817--material-ui-x.netlify.app/x/api/charts/line-chart/#line-chart-prop-loading
| /** | |
| * If `true`, a loading overlay is displayed. | |
| */ | |
| loading: PropTypes.bool, | |
| /** | |
| * If `true`, a loading overlay is displayed. | |
| * @default false | |
| */ | |
| loading: PropTypes.bool, |
| const LoadingText = styled('text')(({ theme }) => ({ | ||
| stroke: 'none', | ||
| fill: theme.palette.text.primary, | ||
| shapeRendering: 'crispEdges', |
There was a problem hiding this comment.
We are missing the font here no?
I would expect to see theme.typography.body1 or .body2 depending on what makes the most sense (maybe body2?)
Fix #10194
Todo
Preview: https://deploy-preview-12817--material-ui-x.netlify.app/x/react-charts/styling/#overlay