-
Notifications
You must be signed in to change notification settings - Fork 826
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
Added scroll_to_widget #483
Conversation
src/textual/_compositor.py
Outdated
@@ -52,17 +52,17 @@ class ReflowResult(NamedTuple): | |||
resized: set[Widget] # Widgets that have been resized | |||
|
|||
|
|||
class RenderRegion(NamedTuple): | |||
class RegionGeometry(NamedTuple): |
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.
A better name...
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.
Awesome! 😊
Once the "integration test" foundation will be merged I guess we could add a test that starts an app with a series of Placeholders, calls this scroll_to_widget
method, waits a bit, and then check that the content did scroll? 🙂
Yeah, that would be great. Hard to unit test some of that logic. |
Co-authored-by: Darren Burns <[email protected]>
return self.scroll_to( | ||
x=self.scroll_target_x + self.container_size.width, animate=animate | ||
) | ||
|
||
def scroll_to_widget(self, widget: Widget, *, animate: bool = True) -> bool: |
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.
If a widget is taller than the container, and you scroll up to it, it scrolls such that the bottom of the widget aligns with the bottom of the container (and the top of the widget is out of view). I suspect we want the top of the widget to be in view.
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.
Just one comment. Also noticed the scroll animation is a constant speed (it feels very slow as well), so if you wrap focus from the bottom to the top it's taking multiple seconds.
Good point. It should be a fixed time. |
I've exposed |
scroll_to_widget
method which scrolls a container to make a widget visible.RenderRegion
toMapGeometry
which is more descriptive.