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

Don't shift timescale bars on barchart #4063

Closed
wants to merge 1 commit into from

Conversation

thymikee
Copy link

Summary

This PR (naively) resolves issue with bars alignment on time scale, by early returning.
Fixes #2415.

Copy link
Member

@simonbrunel simonbrunel left a comment

Choose a reason for hiding this comment

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

I don't think that's the correct approach: the bar controller should not be aware of specific scale types but simply rely on the base scale interface.

Do you have a fiddle that showcase the issue?

@thymikee
Copy link
Author

Here's an example of current behaviour: https://codepen.io/anon/pen/NpzGrZ.
Bars should rather correspond to months, but instead they sit between current and next month.

Copy link
Member

@etimberg etimberg left a comment

Choose a reason for hiding this comment

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

I agree that this PR fixes the issue, but I also agree with @simonbrunel that the bar controller should not care about the type of axis. What this means is that we need to find the proper abstraction between the axes and the controllers to ensure that the type and category axes return the correct information.

@benmccann
Copy link
Contributor

Your problem should hopefully be solved by the recently checked in series option in #4507. Can you give it a try?

I think this PR should be closed now that we have another solution. The bar controller shouldn't care about the time scale.

@thymikee
Copy link
Author

Thanks!

@thymikee thymikee closed this Jul 20, 2017
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.

Bars on time axes are poorly positioned and sized
4 participants