-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Selection on choropleth
to work even if an invisible trace is present
#2099
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
Conversation
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.
Seems reasonable - but I wonder if we should preempt problems like this by just filtering out invisible traces (visible !== true
, to account for legendonly
) here?
Thanks @alexcjohnson, I understand the general direction for preventing the situation from emerging, not sure how I'd go about it as |
@monfera the idea is that the only reason |
This is a much better solution in my opinion. |
src/plots/cartesian/select.js
Outdated
@@ -190,6 +195,7 @@ module.exports = function prepSelect(e, startX, startY, dragOptions, mode) { | |||
selection = []; | |||
for(i = 0; i < searchTraces.length; i++) { | |||
searchInfo = searchTraces[i]; | |||
if(!visible(searchInfo)) 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.
Great find, indeed these are the only callers, and select
check is almost always on the top, except scattermapbox/select/js
where it's okay to set the _hasDimmedPts
to false
anyway:
trace._hasDimmedPts = false;
if(!subtypes.hasMarkers(trace)) return [];
Pushing a new commit now
moved check more upstream on Alex's suggestion |
e757114
to
fc0ad56
Compare
💃 for me. Great PR @monfera ! |
Thanks @etpinard and @alexcjohnson for the added clarity and code size reducing suggestions! |
Box select and lasso to work in the presence of an invisible
choropleth
trace. Updated test case signals if the small fix is commented out.