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

Take into account inside labels of overlaid axes in when hiding labels on the counter axis #5589

Merged
merged 5 commits into from
Apr 21, 2021

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Apr 14, 2021

Addressing #5402.

The autorange padding part was addressed in #5586.
This PR fixes the hiding part.

Demo

@plotly/plotly_js

@archmoj archmoj added bug something broken status: reviewable labels Apr 14, 2021
@archmoj archmoj added this to the NEXT milestone Apr 14, 2021
@nicolaskruchten
Copy link
Contributor

Playing with the codepen, I found that panning the x-axis didn't hide the x-ticklabels when they were below the y-ticklabels... ? I thought we'd fixed this in #5582 ?

@archmoj
Copy link
Contributor Author

archmoj commented Apr 14, 2021

Playing with the codepen, I found that panning the x-axis didn't hide the x-ticklabels when they were below the y-ticklabels... ? I thought we'd fixed this in #5582 ?

Good catch. We did fix it and it is now working as expected without overlaid axis.
So this one appears to be a another piece which I am going to address in this PR.
Thanks!

@archmoj
Copy link
Contributor Author

archmoj commented Apr 16, 2021

This PR is ready for review.

@archmoj archmoj requested a review from alexcjohnson April 16, 2021 23:46
@alexcjohnson
Copy link
Collaborator

@archmoj a few issues when I'm playing with this, using the ticklabelposition-overlay2 mock:

  • Why is there so much padding? Both sides of the axes with inside labels get way more padding than they need.
  • Autorange gets confused with this mock: if I doubleclick one of the subplots it autoranges as if there were no labels inside, then doubleclicking again puts it back to mega-padding.
  • relayout doesn't work: eg Plotly.relayout(gd,'xaxis2.ticklabelposition',null) moves the tick labels but does not update yaxis3.range. I suspect this is related to the autorange confusion

None of these behaviors happen with only one set of inside labels on a given subplot; in fact if I remake this mock with either xaxis2 or xaxis3 having outside labels, the padding is correct, autorange works correctly, and relayout to move the other xaxis inside and out (toggling between zero and one set of inside labels on the top subplot) correctly updates yaxis3.range.

@archmoj
Copy link
Contributor Author

archmoj commented Apr 21, 2021

@archmoj a few issues when I'm playing with this, using the ticklabelposition-overlay2 mock:

  • Why is there so much padding? Both sides of the axes with inside labels get way more padding than they need.

Please note that the axes have specific ranges and they are not autoranged.

@archmoj
Copy link
Contributor Author

archmoj commented Apr 21, 2021

  • Autorange gets confused with this mock: if I doubleclick one of the subplots it autoranges as if there were no labels inside, then doubleclicking again puts it back to mega-padding.

The mega-padding question here is related to the initial view which is controlled by a use-defined range not autorange.

@alexcjohnson
Copy link
Collaborator

they are not autoranged

What's the point of that mock then? Anyway the second point stands, I can't get it to autorange correctly via GUI interactions no matter what I do.

@archmoj
Copy link
Contributor Author

archmoj commented Apr 21, 2021

they are not autoranged

What's the point of that mock then?

To lock the bug. Without that the labels (namely -1 and 1) won't hide under inside ticklabels of the counter axis.

should_hide_ticks_below_inside_labels

@archmoj
Copy link
Contributor Author

archmoj commented Apr 21, 2021

they are not autoranged

Anyway the second point stands, I can't get it to autorange correctly via GUI interactions no matter what I do.

I agree. That looks like a separate bug outside the scope of this PR.

@alexcjohnson
Copy link
Collaborator

Without that the labels (namely -1 and 1) won't hide under inside ticklabels

Ah, got it! Sorry, I misunderstood the purpose here. Yes, please consider autorange a separate bug, which indeed is independent of how many inside-labeled axes are drawn on the subplot: if you start out autoranged everything is fine, but if you start with defined ranges or if you start autoranged and then zoom, doubleclicking will do strange things (and will toggle between two states even if you started out autoranged)

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.

💃 This looks good re: label visibility, apologies for the confusion.

@archmoj archmoj merged commit 3becfb8 into master Apr 21, 2021
@archmoj archmoj deleted the fix5402-hide-overlapping-inside-labels branch April 21, 2021 17:21
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.

3 participants