-
Notifications
You must be signed in to change notification settings - Fork 810
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
Chaining click events (double/triple click etc) #5369
Conversation
…clicks itself [no ci]
…s purpose and behavior
…m typing_extensions
… on_mount method to LazyApp for better lifecycle management in tests
src/textual/app.py
Outdated
@@ -4325,10 +4355,12 @@ def suspend(self) -> Iterator[None]: | |||
# app, and we don't want to have the driver auto-restart | |||
# application mode when the application comes back to the | |||
# foreground, in this context. | |||
# fmt: off |
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.
Ruff formats this code into parentheses, and you can't use parentheses in context managers on 3.8.
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.
Fixed by adding:
[tool.ruff]
target-version = "py38"
To pyproject.toml
.
@@ -556,8 +557,88 @@ class Click(MouseEvent, bubble=True): | |||
|
|||
- [X] Bubbles | |||
- [ ] Verbose | |||
|
|||
Args: |
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've copied the docstring format from other classes in this module, assuming inheritance etc. rather than copy pasting a huge docstring several times.
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.
Some comments for your consideration.
You can simulate double and triple clicks by setting the `times` parameter. | ||
|
||
```python | ||
await pilot.click(Button, times=2) # Double click |
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.
We are calling times
here and chain
on the event?
I see why. It reads like "click button 2 times". But it feels inconsistent.
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 agree it's a bit inconsistent but I think of the event being a lower level technical thing and as you said pilot interacting from the user POV.
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.
How about a pilot.double_click
and pilot.triple_click
(in addition to the times
parameter?
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.
Added these
chain
attribute onClick
events.pilot.click
with atimes
argument, allowing double/triple/etc clicks.This PR doesn't implement any features which make use of double/triple clicks.
Please review the following checklist.