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

revise box & violin hover labels - improve order & handle duplicates #6189

Merged
merged 12 commits into from Jun 14, 2022

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented May 12, 2022

@plotly/plotly_js
cc: @sstripps1

@sstripps1
Copy link

Thank you so much for this!

@alexcjohnson
Copy link
Contributor

Definite improvment! But there may still be cases to investigate... If I load box_plot_jitter_edge_cases (and call Plotly.relayout(gd,'hovermode','x') because otherwise I can't get any hover as the boxes have no size!) two issues occur to me:

(1) median should go in the middle, but it appears first:
Screen Shot 2022-05-20 at 14 30 55

(2) What determines whether upper fence and lower fence labels are included? In the first two traces upper fence appears but not lower fence. In the latter two (as in the image above) neither appears. Maybe they're included only when they're different from max or min respectively? I'd think based on the goal of this PR we should always show all of these labels - ie min, lower fence, q1, median (and mean if it's displayed), q3, upper fence, max. The only exception I can think of is boxpoints=false when we extend the fences to min and max so we shouldn't show upper fence or lower fence.
Screen Shot 2022-05-20 at 14 30 46

@archmoj
Copy link
Contributor Author

archmoj commented May 26, 2022

(2) What determines whether upper fence and lower fence labels are included? In the first two traces upper fence appears but not lower fence. In the latter two (as in the image above) neither appears. Maybe they're included only when they're different from max or min respectively? I'd think based on the goal of this PR we should always show all of these labels - ie min, lower fence, q1, median (and mean if it's displayed), q3, upper fence, max. The only exception I can think of is boxpoints=false when we extend the fences to min and max so we shouldn't show upper fence or lower fence. Screen Shot 2022-05-20 at 14 30 46

This part is addressed in f726248 and e189c4d.

@archmoj
Copy link
Contributor Author

archmoj commented May 31, 2022

Definite improvment! But there may still be cases to investigate... If I load box_plot_jitter_edge_cases (and call Plotly.relayout(gd,'hovermode','x') because otherwise I can't get any hover as the boxes have no size!) two issues occur to me:

(1) median should go in the middle, but it appears first: Screen Shot 2022-05-20 at 14 30 55

This is addressed in e2f6d0c.

@archmoj
Copy link
Contributor Author

archmoj commented Jun 14, 2022

@alexcjohnson this PR is ready for another round of review. Thank you 🙏

Copy link
Contributor

@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.

💃 Very nice!

archmoj and others added 2 commits June 14, 2022 12:29
Co-authored-by: Alex Johnson <alex@plot.ly>
Co-authored-by: Alex Johnson <alex@plot.ly>
@archmoj archmoj merged commit b2df371 into master Jun 14, 2022
@archmoj archmoj deleted the improve-box-violin-hoverlabels branch June 14, 2022 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants