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(Axis): tick index = post-filtered index #818

Merged
merged 3 commits into from
Sep 30, 2020
Merged

Conversation

williaster
Copy link
Collaborator

🐛 Bug Fix

Fixes #815 .

This was a regression introduced in #779 / 0.0.199 where renderTicks was factored out to allow for an animated variant. We had to pass tickLabelProps to renderTicks as an array, one for each tick value. However tick index corresponded to the pre-filtered index, which does not match the tickLabelProps array index when hideZero=true. This was most noticeable when there were no tickLabelProps for the final tick value (see example below), but in reality all of the styles were off by an index of +1.

A simplification of the problem:

const tickValues = [0, 1, 2];
const filteredTickValues = [{ value: 1, index: 1 }, { value: 2, index: 2 }]; // hideZero filtered
const tickLabelProps = [tickLabelPropsFn(1, 1), tickLabelPropsFn(2, 2)];

// renderTicks
...
const tickValue = { value: 2, index: 2 };
const tickProps = tickLabelProps[tickValue.index]; // doesn't exist
...

Before

After

@kristw @hshoff

@coveralls
Copy link

coveralls commented Sep 29, 2020

Pull Request Test Coverage Report for Build 21

  • 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.02%) to 54.34%

Totals Coverage Status
Change from base Build 269438902: -0.02%
Covered Lines: 1099
Relevant Lines: 1976

💛 - Coveralls

@williaster williaster merged commit db9c81e into master Sep 30, 2020
@williaster williaster deleted the chris--fix-axis-ticks branch September 30, 2020 03:17
@williaster williaster added this to the 1.1.0 milestone Oct 17, 2020
@coveralls
Copy link

coveralls commented Nov 23, 2024

Pull Request Test Coverage Report for Build 279304112

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 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.03%) to 54.34%

Totals Coverage Status
Change from base Build 269438902: -0.03%
Covered Lines: 1099
Relevant Lines: 1976

💛 - Coveralls

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

Successfully merging this pull request may close these issues.

AxisLeft: hideZero breaks ticks formatting
3 participants