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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[axis] add AxisRadial component #404

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

williaster
Copy link
Collaborator

馃殌 Enhancements

This PR

  • adds a new <AxisRadial /> component to the @vx/axis package
  • adds propType descriptions and tests for <AxisRadial />
  • updates the LineRadial demo tile to also include the <AxisRadial /> component:

.map((val, index) => {
if (hideZero && val === 0) return null;

const angle = scale(val) - Math.PI / 2;
Copy link
Collaborator Author

@williaster williaster Jan 7, 2019

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;
Copy link
Collaborator Author

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}>
Copy link
Collaborator Author

@williaster williaster Jan 7, 2019

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}
Copy link
Member

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?

Copy link
Collaborator Author

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.

@hshoff hshoff added this to the v0.0.184 milestone Jan 8, 2019
@hshoff hshoff modified the milestones: v0.0.184, v0.0.185, v0.0.186 Jan 28, 2019
@hshoff hshoff removed this from the v0.0.186 milestone Feb 28, 2019
@MrBlenny
Copy link

MrBlenny commented Jul 4, 2021

@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.
Screenshot from 2021-07-03 12-03-07
I've tried getting this working the the latest src but it looks like a bunch of the utils have been removed.
eg)

import polarToCartesian from '../utils/polarToCartesian';
import radialLabelTransform from '../utils/radialLabelTransform';
import radialTickLabelTransform from '../utils/radialTickLabelTransform';
import identity from '../utils/identity';

Is there some way I can help in order to get this merged?

Edit:
I've manually copied this code and the your new utils into my codebase and tested it with visx 1.14.0. Seems to work well and has let me make the gauges I need. i.e.
Screenshot from 2021-07-04 14-05-06

@williaster
Copy link
Collaborator Author

hey @MrBlenny 馃憢 the gauge looks great! 馃槏

This PR was never completed/I lost bandwidth to get it merged. @sarathps93 implemented the radial versions of @visx/grid in #1007 , I'm not sure if this is still on his radar or not. If not, and you are interested in contributing this for others to use, I would happily review a PR!

Comment on lines +19 to +29
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';
}

Choose a reason for hiding this comment

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

use switch case

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants