-
-
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] Create color legend #13700
[charts] Create color legend #13700
Conversation
Deploy preview: https://deploy-preview-13700--material-ui-x.netlify.app/ Updated pages: |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
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 PR, I think most of my comments are minor details.
A big question though is if we shouldn't only have the ChartsLabel
component and derive all other components from it. So the legend
props would have all the configuration necessary
I feel the DX would be simpler since users wouldn't need to use composition if they don't want to? WDYT?
I started with a middle ground: a The issue is that customization props are not the same (length, thikness, minLabel, spagin, itemWidth, ...) so it would be a pain for the API documentation due to props used in some cases, but not others. Since those legends are for already advanced use cases, I preferred to add them in a composition manner. But the heatmap could have it as a slot |
This PR might break the ci because this other pr was merged too #13822 |
Fix #12379
In this PR I propose to add 2 components to display the legend associated with
colorMap
:The
PiecewiseColorLegend
is quite similar to the series legend. So I abstracted the rendering intoLegendPerItem
which takes a array of objects{ color, label }
and do the display part. ThenDefaultChartsLegend
, andPiecewiseColorLegend
only have to define the colors and labels they want to displayKnown point of discussion
LegendPerItem
being a copy past of theDefaultChartsLegend
some naming can be weird/not well suited for thePiecewiseColorLegend
Charts
I thought it would make the naming too long. Buit at the same time not putting it kind of break the convention we follow for charts subcomponents