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

Visible widgets inside an invisible container are not part of the focus chain #3053

Closed
davep opened this issue Aug 3, 2023 · 1 comment · Fixed by #3070
Closed

Visible widgets inside an invisible container are not part of the focus chain #3053

davep opened this issue Aug 3, 2023 · 1 comment · Fixed by #3070
Assignees
Labels
bug Something isn't working Task

Comments

@davep
Copy link
Contributor

davep commented Aug 3, 2023

Textual supports visible children of an invisible container -- this can be helpful with some approaches to laying things out. However, it seems that the visible children of the invisible container aren't seen as part of the focus chain; they can't be tabbed to with the keyboard.

Example code:

from textual.app import App, ComposeResult
from textual.containers import Horizontal, Vertical
from textual.widgets import Button, Label

class InvisibleContainerFocusApp(App[None]):

    CSS = """
    #invisible {
        visibility: hidden;
    }

    #invisible > * {
        visibility: visible;
    }
    """

    def compose(self) -> ComposeResult:
        with Horizontal():
            with Vertical():
                yield Label("The container for this is visible")
                for _ in range(10):
                    yield Button()
            with Vertical(id="invisible"):
                yield Label("The container for this is invisible")
                for _ in range(10):
                    yield Button()

if __name__ == "__main__":
    InvisibleContainerFocusApp().run()
@davep davep added bug Something isn't working Task labels Aug 3, 2023
davep added a commit to davep/textual-sandbox that referenced this issue Aug 3, 2023
@rodrigogiraoserrao rodrigogiraoserrao self-assigned this Aug 7, 2023
willmcgugan pushed a commit that referenced this issue Aug 28, 2023
…focus chain (#3070)

* Add regression tests for #3053

* Traverse invisible containers when computing focus chain.

At the moment, we were completely bypassing invisible containers which meant that their visible children wouldn't be included in the focus chain.

* Make note of removed property.

* Add regression test for #3071.

* Fix #3071.

* Fix regression test for #3053.

* Optimize computation of focus chain.

Computing the focus chain was relying on the property 'visible' of nodes which may traverse the DOM up to find the visibility of a given node. Instead, we cache the visibility of the nodes we traverse and keep them in a stack, saving some of that computation.
Related issues: #3071
Related comments: #3070 (comment)

* Make test more robust.

* Make test more robust.

* Short-circuit disabled portions of DOM.

If a node is disabled, we will not be focusable, nor will its children, so we can skip it altogether.
Related review comment: https://github.com/Textualize/textual/pull/3070/files#r1300292492

* Simplify traversal.

The traversal code could be simplified after reordering some lines of code.
We also get rid of the visibility stack and instead keep everything in the same stack.
Related comments: #3070 (review)
@github-actions
Copy link

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 Task
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants