Skip to content
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] Add documentation on border radius alternative for BarCharts #12859

Conversation

JCQuintas
Copy link
Member

@JCQuintas JCQuintas commented Apr 19, 2024

As @alexfauquette suggested on the previous PR in order to provide a quick alternative to borderRadius on bar charts, we can simply use css' clipPath property, which should be enough for most regular cases.

@alexfauquette not sure this "resolves" the issue at #12220, what do you think?

https://deploy-preview-12859--material-ui-x.netlify.app/x/react-charts/bars/#border-radius
Screenshot 2024-04-19 at 20 15 59

@JCQuintas JCQuintas added docs Improvements or additions to the documentation component: charts This is the name of the generic UI component, not the React module! labels Apr 19, 2024
@mui-bot
Copy link

mui-bot commented Apr 19, 2024

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

Updated pages:

Generated by 🚫 dangerJS against ffbe421

@ishikachhabraa
Copy link

what if we have stacking series? how will that case be handeled?

@JCQuintas
Copy link
Member Author

what if we have stacking series? how will that case be handeled?

Right now it would apply the inset on every one of the items in the stack. So it wouldn't look very good.

Is adding border radius to a stacking bar chart important to you? We thought about documenting this workaround while we gather more info on this. Can you explain why it is important?

@ishikachhabraa
Copy link

what if we have stacking series? how will that case be handeled?

Right now it would apply the inset on every one of the items in the stack. So it wouldn't look very good.

Is adding border radius to a stacking bar chart important to you? We thought about documenting this workaround while we gather more info on this. Can you explain why it is important?

yes, as i was working one of my projects that required this particular design!

@alexfauquette
Copy link
Member

alexfauquette commented Apr 23, 2024

It can work for stacking if you do not dynamically compute the stacking order and will be broken if the top bar has 0 height

https://codesandbox.io/p/sandbox/confident-danny-c7nsz2?file=%2Fsrc%2FDemo.tsx%3A8%2C16

image

Copy link
Member

@alexfauquette alexfauquette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty nice

If we were able to provide isExtremum and value to the ownerState we could support stacking the same way 🤔

Co-authored-by: Alexandre Fauquette <45398769+alexfauquette@users.noreply.github.com>
Signed-off-by: Jose C Quintas Jr <juniorquintas@gmail.com>
@JCQuintas
Copy link
Member Author

Looks pretty nice

If we were able to provide isExtremum and value to the ownerState we could support stacking the same way 🤔

I already have a working POC at #12834

needs cleanup and adjustments though

@JCQuintas JCQuintas merged commit ddd95ee into mui:master Apr 23, 2024
17 checks passed
@JCQuintas JCQuintas deleted the 12220-charts-how-do-i-customize-edgesradius-on-bar-charts-2 branch April 23, 2024 11:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: charts This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants