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

Giving focus to a widget not scrolled into view doesn't always scroll it into view #4461

Closed
davep opened this issue Apr 29, 2024 · 6 comments · Fixed by #4469
Closed

Giving focus to a widget not scrolled into view doesn't always scroll it into view #4461

davep opened this issue Apr 29, 2024 · 6 comments · Fixed by #4469
Labels
bug Something isn't working

Comments

@davep
Copy link
Contributor

davep commented Apr 29, 2024

Using the following code:

from textual.app import App, ComposeResult
from textual.containers import VerticalScroll
from textual.widgets import Label, Input

class Inner(Label, can_focus=True):
    DEFAULT_CSS = """
    Inner {
        width: 1fr;
        border: solid grey;
        &:focus {
            border: thick green;
        }
    }
    """

class Top(VerticalScroll, can_focus=False):

    def compose(self) -> ComposeResult:
        for n in range(100):
            yield Inner(f"{n}. This is a label that can get focus")

class ScrollFocusIssueApp(App[None]):

    AUTO_FOCUS = "Input"

    def compose(self) -> ComposeResult:
        yield Top()
        yield Input()

    def on_mount(self) -> None:
        self.query_one(Top).scroll_end(animate=False)

if __name__ == "__main__":
    ScrollFocusIssueApp().run()

often, but not always, if I tab or shift+tab from the Input to the Inner that isn't currently visible in the Top widget, the focused widget isn't scrolled into view. A subsequent change of focus "further" in the same direction will then result in the subsequently-focused widget being brought into view.

So, using the above, I can often cause the problem like this:

  1. Run up the application
  2. Press tab
  3. Sometimes, but not always, the focused widget doesn't scroll into view.
  4. If it did scroll into view...
  5. shift+tab twice
  6. For me, without fail, the focused widget isn't scrolled into view
@davep davep added the bug Something isn't working label Apr 29, 2024
davep added a commit to davep/textual-sandbox that referenced this issue Apr 29, 2024
@darrenburns
Copy link
Member

I'm pretty sure this is something to do with the animation that occurs when you focus_next. If you switch off animation where scroll_to_center is called inside the focus logic in screen.py, things seem to work consistently.

@darrenburns
Copy link
Member

I can't reproduce the issue in Elia though in a chat with only a couple of messages - I'd have expected it to exhibit the same behaviour given this MRE - I wonder if it's related to the distance that needs to be scrolled or the number of children.

@TomJGooding
Copy link
Contributor

TomJGooding commented Apr 29, 2024

I've not dived too deep into this yet, but if you add a debug log here:

diff --git a/src/textual/screen.py b/src/textual/screen.py
index b84fbc277..6dd3d0a43 100644
--- a/src/textual/screen.py
+++ b/src/textual/screen.py
@@ -632,6 +632,7 @@ class Screen(Generic[ScreenResultType], Widget):
 
                     def scroll_to_center(widget: Widget) -> None:
                         """Scroll to center (after a refresh)."""
+                        self.log.debug(f"{widget.has_focus = }")
                         if widget.has_focus and not self.screen.can_view(widget):
                             self.screen.scroll_to_center(widget, origin_visible=True)

It looks like the scroll_to_center isn't being called because widget.has_focus is False.

@darrenburns
Copy link
Member

Weird, because I added an "animate=False" inside that same block and it seemed to fix the issue. At least, I think it was in there.

@TomJGooding
Copy link
Contributor

@darrenburns I originally tried the same thing but just happened to reproduce the same issue first try, I think the problem is that the bug is very intermittent. The actual issue seems to be when the Focus message is handled to set has_focus (before or after this scroll_to_center function), at least based on my quick look into this.

Copy link

github-actions bot commented May 1, 2024

Don't forget to star the repository!

Follow @textualizeio for Textual updates.

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

Successfully merging a pull request may close this issue.

3 participants