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

fix(xychart): include zero in scale domains by default #1008

Merged
merged 6 commits into from
Jan 11, 2021

Conversation

williaster
Copy link
Collaborator

@williaster williaster commented Jan 8, 2021

🐛 Bug Fix

This fixes a default behavior issue with XYChart scales, where their domains don't include zero by default if the scale allows. Including zero in the axis domain is generally best practice, and users can still opt out by setting x/yScaleConfig={{ zero: false }}.

Before
This AreaSeries has 3 y-values which are all equal and non-zero. It renders like this because the y-domain is [100, 100]. This was also reported in #1003

After
With the same data + chart config, the y-domain is now [0, 100].

Verified it works for negative values, and horizontal orientation.

🚀 Enhancements

  • Adds a scaleCanBeZeroed typeguard util to @visx/scale (needed for typing these changes)
  • exposes DataProvider's horizontal prop in XYChart (noticed a small TODO during this work)

@hshoff @kristw
cc @RIP21

let xScale = (scaleCanBeZeroed(xScaleConfig)
? createScale({
range: [xMin, xMax],
domain: xDomain as [XScaleInput, XScaleInput],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

noting that the domain was updated from number => ScaleInput (leftover from my comment here)

@coveralls
Copy link

Pull Request Test Coverage Report for Build 472873892

  • 4 of 4 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.006%) to 61.202%

Totals Coverage Status
Change from base Build 466988844: 0.006%
Covered Lines: 1730
Relevant Lines: 2692

💛 - Coveralls

@williaster williaster merged commit 460a0db into master Jan 11, 2021
@williaster williaster deleted the chris--xychart-scale-fix branch January 11, 2021 19:25
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.

3 participants