-
Notifications
You must be signed in to change notification settings - Fork 23
feat: add hasSquareCorners prop to bar chart #263
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
Conversation
|
🎨 Storybook -> https://opensource.adobe.com/react-spectrum-charts/PR-263 |
1 similar comment
|
🎨 Storybook -> https://opensource.adobe.com/react-spectrum-charts/PR-263 |
|
|
||
| // Square radius should have values of 0 | ||
| expect(squareRadius).toEqual( | ||
| expect.objectContaining({ |
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 didn't know about expect.objectContaining. Very cool. The more you know.
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.
It's a good way to only check about the properties you care about, regardless of the rest of the object shape.
| const { type, lineWidth, metric, hasSquareCorners } = props; | ||
| const value = hasSquareCorners ? 0 : Math.max(1, CORNER_RADIUS - getLineWidthPixelsFromLineWidth(lineWidth) / 2); |
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.
Clean and simple
|
Can you add one thing? In |
|
All of our props that have default values, we update the type so these are no longer optional when used within our code. That way you never have to check if something exists, you know it will. I realize for a boolean prop it's less critical but for consistency it's nice even on boolean props. |
|
Lastly can you update the prop table in the docs: https://github.com/adobe/react-spectrum-charts/wiki/Bar. Just add in the new prop. Simple update. |
|
|
🎨 Storybook -> https://opensource.adobe.com/react-spectrum-charts/PR-263 |



Description
Adding
hasSquareCornersprop to allow bar charts to render with square tops.Related Issue
Adobe Jira ticket AN-338678
Motivation and Context
This is needed to allow square tops for an Adobe Product Analytics feature called "Forked Funnel"
How Has This Been Tested?
Tested in unit tests in this request
Screenshots (if appropriate):
Types of changes
Checklist: