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
Add support for range selection to RangeTool
#13855
base: branch-3.5
Are you sure you want to change the base?
Conversation
557b028
to
2535273
Compare
2535273
to
5c8f4b6
Compare
@@ -524,6 +524,13 @@ def __init__(self, *args, **kwargs) -> None: | |||
A shaded annotation drawn to indicate the configured ranges. | |||
""") | |||
|
|||
select_gesture = Enum("pan", "tap", "none", default="none", help=""" |
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.
Is there another word we can use here besides "select"? Concerned about overloading this term and confusion with data selections. start_gesture
maybe?
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 think this question boils down to what RangeTool
actually is, because when I see this name, I ask RangeWhat?Tool
and the answer is always RangeSelectTool
(or RangeSelectionTool
), thus everything in this PR (branch name, PR title, new properties, docstrings, etc.) involve the word select
. I thought about alternatives, but nothing comes close in meaning.
start_gesture
maybe?
To me this is the same as above, start_of_what?_gesture
. Maybe let's go generic and simply say interaction_gesture
? Or perhaps we could be specific with range_selection_gesture
and introduce "range selection" as a well defined term (in a glossary of terms) to make sure that we differentiate different types of selections. To me this is a situation similar to what we have with the term "layout", which on its own is really context dependent and may mean server things: CSS/DOM layout, (our) computed layout (LayoutDOM
and canvas layouts) and perhaps other.
if (!this.model.active) { | ||
return | ||
} | ||
|
||
if (ev.key == "Escape") { | ||
if (this._is_selecting) { | ||
this._stop() | ||
return | ||
} | ||
} |
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.
This seems like it could be a bit more compact
if (!this.model.active) { | |
return | |
} | |
if (ev.key == "Escape") { | |
if (this._is_selecting) { | |
this._stop() | |
return | |
} | |
} | |
if (this.model.active) { | |
if (ev.key == "Escape" && this._is_selecting) { | |
this._stop() | |
} | |
} |
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.
@mattpap I would like to have select_gesture
renamed, but apart from that 👍
This optionally allows users of
RangeTool
to make range selection gesture, typically withpan
gesture. This PR additionally experiments with making this configurable, allowing eitherpan
ortap
(tap
,move
,tap
) gestures, to avoid conflict with other tools.Example of using
pan
gesture to make range selection:Screencast.from.30.04.2024.13.09.49.webm
fixes #13646