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

Scrollbars may trap the mouse if hidden while the user is scrolling #4274

Closed
xavierog opened this issue Mar 8, 2024 · 6 comments · Fixed by #4291
Closed

Scrollbars may trap the mouse if hidden while the user is scrolling #4274

xavierog opened this issue Mar 8, 2024 · 6 comments · Fixed by #4291
Assignees
Labels
bug Something isn't working Task

Comments

@xavierog
Copy link
Contributor

xavierog commented Mar 8, 2024

Current behaviour

If a scrollable widget is programmatically hidden while the user is scrolling by dragging the scrollbar handle, all mouse events are seemingly blocked (they are actually trapped by the invisible scrollbar).

This MRE reproduces the issue by programmatically collapsing a Collapsible that contains a scrollable RichLog:

#!/usr/bin/env python3
from textual.app import App, ComposeResult
from textual.widgets import Label, Collapsible, RichLog

INSTRUCTIONS = """[u]Instructions:[/u]
1. Using your mouse, grab the scrollbar handle of the currently focused Collapsible
2. Move it up and down -- [b]do NOT release the mouse button[/b]
3. A few seconds later, the timer should collapse the Collapsible
4. Release the mouse button
5. Try clicking somewhere else, e.g. on the second collapsible
[u]Result:[/u] the mouse is "trapped" by the hidden scrollbar.
[i]Bonus:[/i] Using Tab and Enter, expand the Collapsible and move your mouse.
"""
FILLER = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit\n'
COLLAPSIBLE_TITLE='Collapse me by pressing Enter while moving the scrollbar with your mouse!'

class MouseTrap(App):
	def compose(self) -> ComposeResult:
		yield Label(INSTRUCTIONS)
		for _ in range(2):
			with Collapsible(title=COLLAPSIBLE_TITLE, collapsed=False):
				richlog = RichLog()
				richlog.styles.height = 15
				richlog.write(FILLER * 150)
				yield richlog

	def on_ready(self) -> None:
		self.set_timer(7, self.collapse)

	def collapse(self) -> None:
		self.query('Collapsible').first(Collapsible).collapsed = True

if __name__ == "__main__":
	app = MouseTrap()
	app.run()

Expectations

The scrollbar handle should be "released" as soon as the scrollbar is hidden so that mouse events resume their normal course.

Textual Diagnostics

Versions

Name Value
Textual 0.52.1 / commit f55610e
Rich 13.7.0

Python

Name Value
Version 3.11.8
Implementation CPython
Compiler GCC 13.2.0
Executable ~/whatever/venv/bin/python3

Operating System

Name Value
System Linux
Release 6.6.15-amd64
Version #1 SMP PREEMPT_DYNAMIC Debian 6.6.15-2 (2024-02-04)

Terminal

Name Value
Terminal Application tmux (3.4)
TERM tmux-256color
COLORTERM truecolor
FORCE_COLOR Not set
NO_COLOR Not set

Rich Console options

Name Value
size width=283, height=58
legacy_windows False
min_width 1
max_width 283
is_terminal False
encoding utf-8
max_height 58
justify None
overflow None
no_wrap False
highlight None
markup None
height None
@Textualize Textualize deleted a comment from github-actions bot Mar 11, 2024
@davep davep added the bug Something isn't working label Mar 11, 2024
@willmcgugan
Copy link
Collaborator

At a guess, I suspect it is the mouse capture system not "releasing" the mouse, if the scrollbar is made invisible.

Feels like an easy fix.

@davep davep added the Task label Mar 11, 2024
@davep davep self-assigned this Mar 12, 2024
@TomJGooding
Copy link
Contributor

I'm sure the maintainers are way ahead of me on this, but posting my findings anyway in case it helps...

The ScrollBar does already release the mouse on a Hide event, but perhaps related is #3460 where what Hide means exactly needs some clarification.

def _on_hide(self, event: events.Hide) -> None:
if self.grabbed:
self.release_mouse()
self.grabbed = None

But more importantly this issue isn't only with scrollbars, but any widget which captures the mouse. For example, if you start a mouse selection with the TextArea which is then hidden, you'll see the same behaviour.

davep added a commit to davep/textual-sandbox that referenced this issue Mar 13, 2024
@davep
Copy link
Contributor

davep commented Mar 13, 2024

Just to confirm things: I think @TomJGooding has perfectly summed up the issue here. The situation presented with the MRE above won't result in a Hide event being posted, which does feed back into #3460.

@davep
Copy link
Contributor

davep commented Mar 13, 2024

Digging deeper, what is interesting is the other contents do get a Hide message, which further suggests that the issue here isn't that Hide isn't getting posted, it's that a scrollbar isn't taken into account when sending such messages. For example, the problem documented here is fixed with:

diff --git a/src/textual/screen.py b/src/textual/screen.py
index f9674a47..1cee5b11 100644
--- a/src/textual/screen.py
+++ b/src/textual/screen.py
@@ -793,6 +793,8 @@ class Screen(Generic[ScreenResultType], Widget):
 
                 for widget in hidden:
                     widget.post_message(Hide())
+                    if widget.vertical_scrollbar is not None:
+                        widget.vertical_scrollbar.post_message(Hide())
 
                 # We want to send a resize event to widgets that were just added or change since last layout
                 send_resize = shown | resized

while likely not the actual fix to go with, I think it does suggest that it isn't down to a specific event not being posted, it's just that it isn't being posted as widely as it should.

davep added a commit to davep/textual that referenced this issue Mar 13, 2024
The idea here being that while scrollbars are attached to widgets, they
exist outwith of the DOM, and so don't seem to take part in the usual flow
of show/hide messaging. This change checks, when posting a show/hide
message, if the widget receiving the message has scrollbars and if it does
the message is also sent to them too.

Fixes Textualize#4274.
Copy link

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

@xavierog
Copy link
Contributor Author

Now that the fix is available in main, I confirm the issue no longer happens on my side (tested with both the MRE and my application). Thanks for your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants