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

Implement ticklabelposition for cartesian subplots and colorbars #5275

Merged
merged 52 commits into from Dec 2, 2020

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Nov 13, 2020

Resolves #5186 by implement ticklabelposition for cartesian subplots as well as colorbars.
and fixes #5262 (in c7a23b4)
and fixes #5301 (in 6e80dc9, 8efa7bd and 6f575a3)

ticklabelposition determines where tick labels are drawn with respect to the axis.
Options are
outside, inside,
outside top, inside top,
outside left, inside left,
outside right, inside right,
outside bottom, inside bottom.

It Has no effect on multicategory axes or when tickson is set boundaries.

Please note that
top or bottom has no effect on x axes or when ticklabelmode is set to period.
Similarly,
left or right has no effect on y axes or when ticklabelmode is set to period.

Demo

@plotly/plotly_js

@nicolaskruchten
Copy link
Member

To check:

  • category axes
  • period positioning

@nicolaskruchten
Copy link
Member

recapping conversation in slack: it's pretty much a core part of the intention of this feature that we take into account the bounding boxes of the labels, rather than hardcoding multiples of the font-height.

@archmoj
Copy link
Contributor Author

archmoj commented Nov 18, 2020

recapping conversation in slack: it's pretty much a core part of the intention of this feature that we take into account the bounding boxes of the labels, rather than hardcoding multiples of the font-height.

Addressed in c262d4b and 74f594d.

@alexcjohnson
Copy link
Collaborator

Looking great! Nice job with the extra autorange, and with removing labels that extend outside the plot. A few things I notice playing with the demo:

  • If I zoom/pan the y axis and change the label length while the x axis is still autoranged, I would have expected the x axis range to recalculate to fit the new labels but it doesn't. Somehow if I double-click TWICE on the x axis at that point it will recalculate, but I'd have wanted it to happen automatically. Here for example is starting with short labels (eg 8k) and making them longer by zooming (8000):

Screen Shot 2020-11-22 at 8 36 56 AM

  • Sometimes as I'm interacting with the plot, the x axis data will appear to lose the extra range for the y labels, but the x axis ticks and gridlines will be correct. I can't detect a clear pattern for when this happens, and sometimes I see just a small shift, not all the way to where the data would have been without the inside labels. This looks similar to a bug I encountered in rework matching & scaleanchor so they work together #5287 and fixed with an extra ax.setScale at the right place in the process

Screen Shot 2020-11-22 at 8 34 11 AM

Co-authored-by: Alex Johnson <johnson.alex.c@gmail.com>
@archmoj archmoj changed the title Implement ticklabelposition for cartesian subplots Implement ticklabelposition for cartesian subplots and colorbars Nov 26, 2020
@archmoj
Copy link
Contributor Author

archmoj commented Dec 1, 2020

  • Sometimes as I'm interacting with the plot, the x axis data will appear to lose the extra range for the y labels, but the x axis ticks and gridlines will be correct. I can't detect a clear pattern for when this happens, and sometimes I see just a small shift, not all the way to where the data would have been without the inside labels. This looks similar to a bug I encountered in rework matching & scaleanchor so they work together #5287 and fixed with an extra ax.setScale at the right place in the process
Screen Shot 2020-11-22 at 8 34 11 AM

Good catch. Fixed by 999f877.

@archmoj
Copy link
Contributor Author

archmoj commented Dec 2, 2020

Looking great! Nice job with the extra autorange, and with removing labels that extend outside the plot. A few things I notice playing with the demo:

  • If I zoom/pan the y axis and change the label length while the x axis is still autoranged, I would have expected the x axis range to recalculate to fit the new labels but it doesn't. Somehow if I double-click TWICE on the x axis at that point it will recalculate, but I'd have wanted it to happen automatically. Here for example is starting with short labels (eg 8k) and making them longer by zooming (8000):
Screen Shot 2020-11-22 at 8 36 56 AM

Good call. Addressed in 445eee4.

else if(bb.top + (ax.tickangle ? 0 : d.fontSize / 4) < min) hide = true;
}
if(hide) thisLabel.select('text').style({ opacity: 0 });
} // TODO: hide mathjax?
Copy link
Collaborator

Choose a reason for hiding this comment

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

heh inside labels and MathJax hopefully have rather disjoint user sets... So yes we should test this at some point, but not necessary for this PR

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

Fantastic! Nice job including colorbars and improving various other aspects of rotated tick label positioning. 💃

@archmoj archmoj merged commit 08f3f04 into master Dec 2, 2020
@archmoj archmoj deleted the ticklabelposition branch December 2, 2020 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants