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(coordinateGridMixin): renderVerticalGridLines should consider useTopXAxis #1819

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

aberenyi
Copy link
Contributor

@aberenyi aberenyi commented Mar 15, 2021

Fixes #1818

@aberenyi
Copy link
Contributor Author

@gordonwoodhull would you mind to review this whenever you have a spare few mins?

@gordonwoodhull
Copy link
Contributor

gordonwoodhull commented Mar 19, 2021

Sorry for the slow response. This makes sense to me and I will merge it next release.

If you would be so kind as to add a test case or two, we can ensure that the new feature keeps working into the future.

If you add a test case that fails without this fix and succeeds with it, I think that will be sufficient to test useTopXAxis. It wouldn't hurt to also test something from the last PR #1816, like _xAxisY() or the translate on the label position.

Thanks!

@aberenyi
Copy link
Contributor Author

@gordonwoodhull no probs at all.

Will try to write some tests in due course.

Cheers

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

Successfully merging this pull request may close these issues.

Vertical grid lines aren't visible when useTopXAxis is true
2 participants