mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
3.79k stars 1.12k forks source link

[charts] Rounded corners on BarChart #12834

Open JCQuintas opened 2 weeks ago

JCQuintas commented 2 weeks ago

Initial Proposal

Docs

https://deploy-preview-12834--material-ui-x.netlify.app/x/react-charts/bars/#border-radius

Screenshot 2024-05-01 at 17 07 55

resolves #12220 resolves #12947

Todo

mui-bot commented 2 weeks ago

Deploy preview: https://deploy-preview-12834--material-ui-x.netlify.app/

Updated pages:

Generated by :no_entry_sign: dangerJS against 7ddb5d7c95d1c5e112c53c4f7d1d2ef2db9f0554

JCQuintas commented 1 week ago

@alexfauquette can you take a look at the code again? Documentation is still missing, but I want to be sure I'm on the right path before documenting the behaviours.

https://codesandbox.io/p/sandbox/mui-mui-x-x-charts-rounded-corners-lvgmlc?file=%2Fsrc%2Fdemo.tsx%3A155%2C22

It took me a while to figure out how the mask could work with the animation, but the trick was to animate everything together 😅, there are some unnecessary masks, but I don't think they should be a problem.

There were two options I could go for in terms of masking

I chose to mask the entire column as I assume it gives more flexibility to the radius, as you can really make the columns round, although you probably shouldn't. If we put the mask only on the last item, if the item is very small or non-existant it would probably break and we would need more logic to figure everything out. Only drawbacks I see is that we have to use one mask for every segment, rather than one for the entire column, but it works and animation is aligned. Another "drawback" but this would be in any solution, is that if the "column" is smaller than the chosen radius, it will "clamp" the radius to the height of the column, which might look odd, but there is no other way around it.

Screenshot 2024-04-25 at 12 49 51

Screenshot 2024-04-25 at 12 50 55

JCQuintas commented 4 days ago

from SVG with the following structure

codesandbox example

It doesn't work if we have multiple charts in the same page that have the same id/order. That was my first attempt actually. But svg ids follow the same rules as HTML, and are unique to the entire page.

I can just use useChartId which generates a unique id for each chart 😅

JCQuintas commented 2 days ago

@noraleonte @LukasTy I believe I addressed most if not all of @alexfauquette points. So please review when possible.