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

Modifying Radar's Axis to be drawn by Axis.domain.line of the Theme #1754

Merged
merged 2 commits into from
Sep 1, 2021

Conversation

GoodGoodJM
Copy link
Contributor

It seems that Radar's Axis is not currently being drawn by referring to the theme incorrectly.
Modify to draw axis using theme.axis.domain.line.

@plouc
Copy link
Owner

plouc commented Sep 1, 2021

@GoodGoodJM, that's expected, this is the equivalent of the cartesian grid, not an axis.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Sep 1, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@plouc
Copy link
Owner

plouc commented Sep 1, 2021

But there's a bug in the current implementation, should be theme.grid.line.

@GoodGoodJM
Copy link
Contributor Author

GoodGoodJM commented Sep 1, 2021

ok but, if use theme.gird.line, it change all line's styles of radar.
If we does not want axis, all line will remove..

@plouc plouc added bug radar @nivo/radar package labels Sep 1, 2021
@plouc
Copy link
Owner

plouc commented Sep 1, 2021

@GoodGoodJM, yes, both are part of the grid, it's the same behavior as other charts, using axis is not correct. We could add another option to disable those.

Copy link
Owner

@plouc plouc left a comment

Choose a reason for hiding this comment

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

LGTM

@plouc plouc merged commit d9213a7 into plouc:master Sep 1, 2021
@plouc
Copy link
Owner

plouc commented Sep 1, 2021

Thank you for this fix @GoodGoodJM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug radar @nivo/radar package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants