-
Notifications
You must be signed in to change notification settings - Fork 691
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鈥檒l occasionally send you account related emails.
Already on GitHub? Sign in to your account
[axis] add AxisRadial
component
#404
base: master
Are you sure you want to change the base?
Conversation
.map((val, index) => { | ||
if (hideZero && val === 0) return null; | ||
|
||
const angle = scale(val) - Math.PI / 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.
Maybe haven't had enough 鈽曪笍 but I don't fully understand why the -馃ェ/2
is needed ... but with different scale ranges (e.g., [-馃ェ, 馃ェ]
, [0, 2*馃ェ]
), the axis was always rotated +45掳 without this.
const dy = toPoint.y - fromPoint.y; | ||
|
||
// offset the label in the same direction as the tick orientation | ||
const x = toPoint.x + dx / 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.
this offset is somewhat arbitrary but gave decent results as you can see in the demo tile. we could possibly make it more configurable.
|
||
if (children) { | ||
return ( | ||
<Group className={cx('vx-axis-radial', axisClassName)} top={top} left={left}> |
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.
all classes are the same as other Axis*
components except this top-level one, which should be sufficient to distinguish from other (linear) axes on the page.
stroke={stroke} | ||
strokeWidth={strokeWidth} | ||
strokeDasharray={strokeDasharray} | ||
curve={curveCardinal} |
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.
does this always have to be curveCardinal
? should it be configurable via prop?
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 think you'd probably want this most of the time, otherwise the axis wouldn't look like an arc.
In thinking about this more though, I changed the implementation of GridRadial
to use Arc
instead of LineRadial
, that could simplify this as well and allow us to drop the @vx/curve
dep. With LineRadial
we're sort of doing the arc interpolation ourself but Arc
should be best at that.
@williaster this looks great! Where did this PR get to? This is exactly what I'm after in order to create some gauge-style components.
Is there some way I can help in order to get this merged? Edit: |
hey @MrBlenny 馃憢 the gauge looks great! 馃槏 This PR was never completed/I lost bandwidth to get it merged. @sarathps93 implemented the |
if (labelOrientation === 'top') { | ||
y = -radius - offset; | ||
} else if (labelOrientation === 'right') { | ||
x = radius + offset; | ||
textAnchor = 'start'; | ||
} else if (labelOrientation === 'bottom') { | ||
y = radius + offset; | ||
} else if (labelOrientation === 'left') { | ||
x = -radius - offset; | ||
textAnchor = 'end'; | ||
} |
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.
use switch case
馃殌 Enhancements
This PR
<AxisRadial />
component to the@vx/axis
packagepropType
descriptions and tests for<AxisRadial />
LineRadial
demo tile to also include the<AxisRadial />
component: