-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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][docs] Add more illustrations to the Overview page #12041
Conversation
Deploy preview: https://deploy-preview-12041--material-ui-x.netlify.app/ Updated pages: |
✅ Deploy Preview for material-ui-x ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Nice, I like the idea of adding a "planned" chip 👍
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.
I love the idea
But with the current design, IMHO if you read to fast it's very easy to miss the "PLANNED" chip and be disappointed once you reach the doc.
Do you think we could have some design diff on the cart itself? (have it gray, of with a slightly lower opacity for instance).
@danilo-leal Make me also think that some charts will be pro and could get an extra UI to show this distinction between MIT and pro |
Great ideas! Just added design treatment to both of these suggestions :) |
Should we change "Sparkline Chart" to "Sparkline"? It's just out-of-reach to make a suggestion on the code. It might also make sense to make the illustrated Sparkline smaller than the Line Chart above it (rather than larger), since they're intended to be small (typically much smaller than the examples in the docs page). That might also make room to also illustrate "Sparkbars" (that curiously are a feature of Sparkline rather than a separate component), although I appreciate that might break the consistency of the illustrations... |
Looking again, I would question whether Gauge, Heatmap and Treemap should have "Chart" in the name; and "Funnel Diagram" seems to be preferred to "Funnel Chart" almost 2:1 (Google search results). "Sankey Diagram" outnumbers "Sankey Chart" by a smaller margin, but is what I'm used to hearing (sample size of one!). And then there's the question of capitalisation of "Chart" (cc @samuelsycamore). Meanwhile we're missing "chart" from the navigation and page titles, with odd pluralisation such as "Bars", but that's a separate issue! 😁 |
On that one I agree, adding "Chart" after Gauge does not feel natural when writing its docs |
Good point—if Google results are any indication of preferred usage, I'd drop it from all three. Seems like "Treemap" rarely has a space in the middle, but "Heatmap" could go either way (although "Heat Map" seems to be used more, but maybe not in this specific context). The most important thing is that we style these consistently across all of the docs. |
Those are great use cases for |
Good call on the writing tweaks! I've pushed the low-hanging ones y'all mentioned already, but there might be other places where we'll need to standardize/polish, which I think we can tackle on a separate PR! On a different note, though, does anyone know why the Netlify deploy gets stuck? The CIs are still running as I write this comment, but I expect it to get stuck in the Netlify and test_e2e_website as before the two commits I pushed above! 🤔 |
Yes I can explain that. Netlify added a security to protect secret token. People that are not recognized as a team member by netlify do not trigger the build process. I unblocked this deployement. Still need to find how to add you in the team |
A quick follow up on #12041
A quick follow up on #12041
@flaviendelangle This looks OK to me on my screen 😄 but fair point. Now, I think there is a more obvious issue, this is not a Treemap: but it was designed https://www.figma.com/file/MfirV6NOFAp2xxAZX1QU1V/Docs-infra?type=design&node-id=2116-11941&mode=design&t=LQ6LTwm4tA04VpD0-0 so we could replace it 😄 cc @alexfauquette do you want to take this one in? |
Oops... I guess I forgot to add the correct image there! 😬 |
A quick follow up on mui#12041
This PR adds illustrations for all of the chart types that we currently have lined up (either planned or not) and also adds the capacity to display them as planned.
https://deploy-preview-12041--material-ui-x.netlify.app/x/react-charts/#all-mui-x-charts-components