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.

Sign up for GitHub

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

Jump to bottom

Unified hover: use legend.tracegroupgap in mock legend #6875

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Unified hover: use legend.tracegroupgap in mock legend #6875

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62b3425
Unified hover: use legend.tracegroupgap in mock legend instead of har…
vladsavelyev Feb 1, 2024
150cce3
Add draft log
vladsavelyev Feb 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions draftlogs/6875_change.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Unified hover: use `legend.tracegroupgap` in mock legend instead of setting a hardcoded value [[#6864](https://github.com/plotly/plotly.js/pull/6864)]
2 changes: 1 addition & 1 deletion src/components/fx/hover.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,7 @@ function createHoverText(hoverData, opts) {
bgcolor: hoverlabel.bgcolor,
bordercolor: hoverlabel.bordercolor,
borderwidth: 1,
tracegroupgap: 7,
tracegroupgap: fullLayout.legend ? fullLayout.legend.tracegroupgap : undefined,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm I see we have the same logic on the next line for traceorder but this most likely predates multiple legends. I suppose this is going to give the correct behavior 99% of the time, but at some point we may want to look for which legend say the first trace shown in hover is included in, and use the tracegroupgap from that one.

Do we want to fall back on undefined or would the previous default of 7 be better if there's no legend at all? Or maybe change it to 10, which is the default tracegroupgap when we DO have a legend, so adding an unstyled legend won't change the behavior here?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vladsavelyev any thoughts about @alexcjohnson 's comment? thanks - @gvwilson

traceorder: fullLayout.legend ? fullLayout.legend.traceorder : undefined,
orientation: 'v'
}
Expand Down