-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Fix interacting with selection on subplots with overlaid axes #6870
Conversation
if(xAxisIds.indexOf(trace.xaxis) === -1) continue; | ||
if(yAxisIds.indexOf(trace.yaxis) === -1) continue; | ||
if(xAxisIds.indexOf(trace.xaxis) === -1 && (!trace._xA || !trace._xA.overlaying)) continue; | ||
if(yAxisIds.indexOf(trace.yaxis) === -1 && (!trace._yA || !trace._yA.overlaying)) continue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very interesting proposed fix.
It looks like we also select traces in other subplots that are not overlay with these axes.
If that's the case we shouldn't include them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have checked all the other test cases with subplots and the selection seems to be working fine.
The only cases I've found where there are some crossover with selection/deselection are with:
plot_types
plot_types_grid_dash
box_plot_jitter_edge_cases
mapbox_choropleth-multiple
And for these the issues happen with and without my changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Farkites Thanks very much for your tests.
Here we need to know if we have two subplots i.e. (xy + xy2 ) and (x2y3 + x2,y4) and the user reselects over the first one; then are the traces in the second subplot (x2y3) are also re-selected before and after this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@archmoj Yes, in that scenario the traces in the second subplot (x2y3) are re-selected before and after the changes. You can check this here:
- Before changes: https://codepen.io/Mario-Rodr-guez-Ib-ez/pen/ExMEbdK
- After changes: https://codepen.io/Mario-Rodr-guez-Ib-ez/pen/QWoQoao
Thanks very much for all these manual tests and the brilliant fix. 💃 💃 Please open a follow up PR to include jasmine tests to lock down the bug. |
@Farkites One more request before merging this. |
Add Jasmine Test for bugfix #6870
Fixes #6530
Description
Extending an existing box/lasso selection on traces that overlay axes don't update selection. Only the first trace (which doesn't have
overlay
is updated). See video below:https://github.com/plotly/plotly.js/assets/35932204/91f75734-f47d-47b6-81b0-806413b2cf45
This was happening because the
yAxisIds
is equal to'y'
andtrace.yaxis
to y + the index of the trace (e.gy4
), soyAxisIds.indexOf(trace.yaxis)
is-1
for all the overlaying traces.