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

Multicategory inside ticks fixes #3326

Merged
merged 3 commits into from Dec 14, 2018

Conversation

Projects
None yet
3 participants
@etpinard
Copy link
Member

etpinard commented Dec 12, 2018

Follow-up on #3300. Two multicategory fixes discovered when trying to clean up our ax._boundingBox mess.

Edit: This PR fixes one more bug: https://codepen.io/nicolaskruchten/pen/roOzQO?editors=0010

cc @plotly/plotly_js

etpinard added some commits Dec 12, 2018

use labelLength to position multicategory ax titles
... replacing (buggy) ax._boundingBox. This fixes ax title placement
    on multicategory axes with mirror ticks
fix multicategory axes with inside ticks
... where dividers and ticks go in opposite directions
@etpinard

This comment has been minimized.

Copy link
Member Author

etpinard commented Dec 13, 2018

A new era of reviews is starting now. Asking @archmoj and @antoinerg to review this thing!

@archmoj
Copy link
Collaborator

archmoj left a comment

@etpinard Great fix.
My comments are non-blocking.

@archmoj

This comment has been minimized.

Copy link
Collaborator

archmoj commented Dec 14, 2018

Nicely done 💃

@etpinard

This comment has been minimized.

Copy link
Member Author

etpinard commented Dec 14, 2018

Nice job reviewing! 🎉

@etpinard etpinard merged commit 9bbe295 into master Dec 14, 2018

8 checks passed

ci/circleci: build Your tests passed on CircleCI!
Details
ci/circleci: publish Your tests passed on CircleCI!
Details
ci/circleci: test-image Your tests passed on CircleCI!
Details
ci/circleci: test-image2 Your tests passed on CircleCI!
Details
ci/circleci: test-jasmine Your tests passed on CircleCI!
Details
ci/circleci: test-jasmine2 Your tests passed on CircleCI!
Details
ci/circleci: test-syntax Your tests passed on CircleCI!
Details
continuous-integration/appveyor/branch AppVeyor build succeeded
Details

@etpinard etpinard deleted the multicategory-inside-ticks branch Dec 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.