Skip to content
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

prevent stuck scrollbar #2003

Merged
merged 4 commits into from
Mar 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

- Fixed bug that prevented pilot from pressing some keys https://github.com/Textualize/textual/issues/1815
- DataTable race condition that caused crash https://github.com/Textualize/textual/pull/1962
- Fixed scrollbar getting "stuck" to cursor when cursor leaves window during drag https://github.com/Textualize/textual/pull/1968
- Fixed scrollbar getting "stuck" to cursor when cursor leaves window during drag https://github.com/Textualize/textual/pull/1968 https://github.com/Textualize/textual/pull/2003
- DataTable crash when enter pressed when table is empty https://github.com/Textualize/textual/pull/1973

## [0.13.0] - 2023-03-02
Expand Down
54 changes: 31 additions & 23 deletions src/textual/driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ def __init__(
self._size = size
self._loop = asyncio.get_running_loop()
self._mouse_down_time = _clock.get_time_no_wait()
self._dragging = False
self._dragging_button = None
self._down_buttons: list[int] = []
self._last_move_event: events.MouseMove | None = None

@property
def is_headless(self) -> bool:
Expand All @@ -44,29 +44,37 @@ def process_event(self, event: events.Event) -> None:
"""Performs some additional processing of events."""
if isinstance(event, events.MouseDown):
self._mouse_down_time = event.time
if event.button:
self._down_buttons.append(event.button)
elif isinstance(event, events.MouseUp):
if event.button:
self._down_buttons.remove(event.button)
elif isinstance(event, events.MouseMove):
if event.button and not self._dragging:
self._dragging = True
self._dragging_button = event.button
elif self._dragging and self._dragging_button != event.button:
# Artificially generate a MouseUp event when we stop "dragging"
self.send_event(
MouseUp(
x=event.x,
y=event.y,
delta_x=event.delta_x,
delta_y=event.delta_y,
button=self._dragging_button,
shift=event.shift,
meta=event.meta,
ctrl=event.ctrl,
screen_x=event.screen_x,
screen_y=event.screen_y,
style=event.style,
if (
self._down_buttons
and not event.button
and self._last_move_event is not None
):
buttons = list(dict.fromkeys(self._down_buttons).keys())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this the same as list(self._down_buttons)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, it will additionally unique-ify them. Wether that is strictly necessary, I'm not certain.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right. In that case, why not just buttons = set(self._down_buttons) and then iterate over the set?
Or do we need to preserve order?

It makes a bit more sense, it is probably faster, and doesn't require a comment explaining the hoops you are going over.
Otherwise, I think this warrants a comment.

Depending on what you reply to this comment, I don't mind PR'ing the change.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is intended to preserve order. So that any button ups, arrive in the same order as they went down. Feel free to add a comment.

self._down_buttons.clear()
move_event = self._last_move_event
for button in buttons:
self.send_event(
MouseUp(
x=move_event.x,
y=move_event.y,
delta_x=0,
delta_y=0,
button=button,
shift=event.shift,
meta=event.meta,
ctrl=event.ctrl,
screen_x=move_event.screen_x,
screen_y=move_event.screen_y,
style=event.style,
)
)
)
self._dragging = False
self._dragging_button = None
self._last_move_event = event

self.send_event(event)

Expand Down
8 changes: 6 additions & 2 deletions src/textual/scrollbar.py
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,7 @@ def _render_bar(self, scrollbar_style: Style) -> RenderableType:
def _on_hide(self, event: events.Hide) -> None:
if self.grabbed:
self.release_mouse()
self.grabbed = None

def _on_enter(self, event: events.Enter) -> None:
self.mouse_over = True
Expand All @@ -299,10 +300,12 @@ def _on_leave(self, event: events.Leave) -> None:
self.mouse_over = False

def action_scroll_down(self) -> None:
self.post_message(ScrollDown() if self.vertical else ScrollRight())
if not self.grabbed:
self.post_message(ScrollDown() if self.vertical else ScrollRight())

def action_scroll_up(self) -> None:
self.post_message(ScrollUp() if self.vertical else ScrollLeft())
if not self.grabbed:
self.post_message(ScrollUp() if self.vertical else ScrollLeft())

def action_grab(self) -> None:
self.capture_mouse()
Expand All @@ -313,6 +316,7 @@ def action_released(self) -> None:
async def _on_mouse_up(self, event: events.MouseUp) -> None:
if self.grabbed:
self.release_mouse()
self.grabbed = None
event.stop()

def _on_mouse_capture(self, event: events.MouseCapture) -> None:
Expand Down