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

Axes line w/o ticklabels fix #1910

Merged
merged 4 commits into from
Jul 25, 2017
Merged

Axes line w/o ticklabels fix #1910

merged 4 commits into from
Jul 25, 2017

Conversation

etpinard
Copy link
Contributor

fixes #1907

which was broken since eb48a9d where on this line ax.tickfont is assumed to exist - which isn't the case when showticklabels is false.

Commit 3c26cbf first adds a mock inspired by https://codepen.io/plotly/pen/qXWmpY and generates a baseline using v1.28.3 (i.e before the regression).

Commit fe20694 proposes a fix where a fallback for tickfont.size is added, similar to another fallback in axes.js. Notice that commit fe20694 did generate a diff for the new mock, but looks like it is only of a few sub-pixels:

peek 2017-07-24 11-57

cc @alexcjohnson

@etpinard etpinard added status: reviewable bug something broken labels Jul 24, 2017
@alexcjohnson
Copy link
Collaborator

Thanks for ferreting this out @etpinard

In the case where this fallback is needed, labelStandoff doesn't actually get used, right? So can we just include && ax.showticklabels in the conditional?

@etpinard
Copy link
Contributor Author

In the case where this fallback is needed, labelStandoff doesn't actually get used, right? So can we just include && ax.showticklabels in the conditional?

Good call. Done in d951f6a

@alexcjohnson
Copy link
Collaborator

💃 🎉

'$(docker inspect --format \'{{.Id}}\' ' + constants.testContainerName + ')',
'lxc-attach',
'-n', containerId,
'-f', '/var/lib/docker/containers/' + containerId + '/config.lxc',
Copy link
Contributor Author

@etpinard etpinard Jul 25, 2017

Choose a reason for hiding this comment

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

🔈 Big shoutout to hasimo on the CircleCI forums:

https://discuss.circleci.com/t/container-is-not-defined-on-lxc-attach/14684/5

Copy link
Collaborator

Choose a reason for hiding this comment

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

🥇 🎺 lets get back in business!

@etpinard etpinard merged commit d22b650 into master Jul 25, 2017
@etpinard etpinard deleted the axes-line-noticklabels-fix branch July 25, 2017 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

showline: true while showticklabels: false breaks plot
2 participants