You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
IMHO two parameters were not named clearly, so renaming
line_props -> handle_props
maxdist -> handle_grab_distance
Minor improvements:
document default for buttons
document default for drag_from_anywhere
properly document onselect
Heads up: I also see a naming issue with interactive (see #20113 (comment)), but it's unclear if we should go back to the old span_stay or a different new name. To be discussed in tomorrows dev call. This can be addressed separately from this PR.
Indeed, the new names are much better, would it make sense to also rename these for RectangleSelector/EllipseSelector?
I agree with #20113 (comment) that the name doesn't describe completely what it does. However, persistent is not good enough, as was span_stays, because it doesn't describe the fact that the selector can be changed "interactively" using the handle.
The motivation in renaming it to interactive in #20113 was to improve the consistency with RectangleSelector/EllipseSelector.
Maybe a better way to do this is to add persistent alongside interactive_handle (or something similar)? This will allow the case where the selector patch stays on the axis (is persistent) but is not interactive? interactive_handle=True will need to set persistent=True. Even if this makes the API of the selector slightly more complicated by adding more argument, the meaning of the arguments are more clear!
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
Follow up to #20113.
line_props->handle_propsmaxdist->handle_grab_distancebuttonsdrag_from_anywhereonselectHeads up: I also see a naming issue with
interactive(see #20113 (comment)), but it's unclear if we should go back to the oldspan_stayor a different new name. To be discussed in tomorrows dev call. This can be addressed separately from this PR.