Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fixup labels on reversed axes when ticklabelmode is set to period #5060

Merged
merged 2 commits into from
Aug 12, 2020

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Aug 10, 2020

Follow up of implementing #4911 by #4993.
Fixes the labels on reversed axes the bug introduced in 83f5cdd.

@plotly/plotly_js

@archmoj archmoj added bug something broken status: reviewable labels Aug 10, 2020
@alexcjohnson
Copy link
Collaborator

alexcjohnson commented Aug 11, 2020

Nice, good catch!
testing this out this seems to introduce a (much more minor) bug with regular non-reversed axes when the tick corresponding to the first tick label is just out-of-range, we get the full 2-line label twice:

Screen Shot 2020-08-11 at 5 56 06 PM

It'd be nice to include jasmine tests for ^^ both forward and reversed, with the original range shifted in something like 6-hour increments - so ranges:

['2019-12-29', '2020-01-04']
['2019-12-29 06', '2020-01-04 06']
['2019-12-29 12', '2020-01-04 12']
['2019-12-29 18', '2020-01-04 18']
['2020-01-04 18', '2019-12-29 18']
['2020-01-04 12', '2019-12-29 12']
['2020-01-04 06', '2019-12-29 06']
['2020-01-04', '2019-12-29']

And just check that the tick text we get in each case is as expected. Something like that should cover all the related cases, I'd think.

@archmoj
Copy link
Contributor Author

archmoj commented Aug 12, 2020

@alexcjohnson in order not to intersect with #5065, I need to merge this PR and work on the fix for #5065 (comment).
I could address your comment above afterwards.
So merging...

@archmoj archmoj merged commit 29b456e into master Aug 12, 2020
@archmoj archmoj deleted the ticklabel-period-reversed branch August 12, 2020 13:49
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants