-
Notifications
You must be signed in to change notification settings - Fork 38
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
Set Bqplot*Modes to observe only one selected
state in BrushSelector
#419
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #419 +/- ##
==========================================
- Coverage 86.63% 86.62% -0.02%
==========================================
Files 89 89
Lines 5163 5159 -4
==========================================
- Hits 4473 4469 -4
Misses 690 690 ☔ View full report in Codecov by Sentry. |
Won't all the calls to |
|
I won't have time for to test this week. Try Cubeviz example notebook if you cannot wait. I could reproduce it there (without this patch) but not consistently. |
Tried on 3 different macOS and Python versions now, with notebook, jupyterlab and standalone for Cubeviz and the original Imviz example, and never saw as much as a flicker in size – will have to wait then. |
@cshanahan1 , since you most recently ran into the "disappearing subset" problem, do you want to see if this PR helps? Given how non-deterministic it is, better have multiple devs confirm. |
Maaaybe this fixes some of the problem, but it does not prevent the case where user is dragging something around very fast with repeated down/up clicks. The cursor can accidentally be right at the edge, where the "resize" mode would activate. Then when user intends to move, the app thinks user is resizing, resulting in the shape getting very big or small. |
Despite #419 (comment) , listening to just one thing is better than having race condition, right? Should you take this out of draft? |
Cursor being too close to the edge so it does switch into resize mode seems out of scope for the race condition issue. |
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 sensible, thanks!
Description
Pushing this as a test to avoid possible race conditions in observing both
selected_x
andselected_y
in the BrushSelector interaction that have been speculated to cause unwanted size changes of the selection in #418.Since the reported bug is non-deterministic and I have not been able to reproduce it at all, I have no idea if this change will help, just keeping it here for further investigation.