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 scale options in xychart #987

Merged
merged 1 commit into from
Jan 4, 2021
Merged

Conversation

Janpot
Copy link
Contributor

@Janpot Janpot commented Jan 3, 2021

🐛 Bug Fix

Fixes scale options not applying in XYChart. See #986

zero, nice,... options are applied in createScale. they were overwritten by setting the domain and range afterwards. this PR makes sure the domain and range are set in the createScale call directly so that other options are applied in the correct order.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 459497485

  • 2 of 2 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.08%) to 61.176%

Totals Coverage Status
Change from base Build 441105785: -0.08%
Covered Lines: 1728
Relevant Lines: 2690

💛 - Coveralls

@Janpot Janpot marked this pull request as ready for review January 3, 2021 22:38
@williaster williaster added this to the 1.4.0 milestone Jan 4, 2021
Copy link
Collaborator

@williaster williaster left a comment

Choose a reason for hiding this comment

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

@Janpot thanks for the bug report and fix 🙏 ❤️

I encountered this myself before the holidays but didn't quite have time to submit a fix, this LGTM 🎉

xScale.domain(xScaleConfig.domain || xDomain);
let xScale = createScale({
range: [xMin, xMax],
domain: xDomain as [number, number],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think depending on scale type this won't always be a number, but I can try to fix the typing in a followup PR as types can be tricky to debug here.

@williaster williaster linked an issue Jan 4, 2021 that may be closed by this pull request
@williaster williaster merged commit 7f678d9 into airbnb:master Jan 4, 2021
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.

[xychart] x/yscale config for zero/nice has no effect
3 participants