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

Selecting the root of a Tree does not highlight descendants. #2397

Closed
davep opened this issue Apr 27, 2023 · 6 comments · Fixed by #2482
Closed

Selecting the root of a Tree does not highlight descendants. #2397

davep opened this issue Apr 27, 2023 · 6 comments · Fixed by #2482
Assignees
Labels
bug Something isn't working Task

Comments

@davep
Copy link
Contributor

davep commented Apr 27, 2023

The Tree widget will highlight the lines that descend from the selected node, but at the moment it doesn't seem to do it from the top level. For example, consider this:

Screenshot 2023-04-27 at 10 31 00

where the topmost node is selected but no lines are highlighted, against this:

Screenshot 2023-04-27 at 10 31 09

where a node one level down is highlighted and all the line below are highlighted.

We should review this, decide exactly what is supposed to happen, and have it be the same for all levels.

Also @darrenburns has raised the extra question of if it might be an idea to have it such that it only highlights the immediate lines vs all the lines in the tree below.

@davep davep added enhancement New feature or request Task labels Apr 27, 2023
@willmcgugan
Copy link
Collaborator

The expected behaviour is to highlight all descendants, including the root.

Let's fix that first.

@willmcgugan willmcgugan changed the title Review the line highlighting in the Tree Selecting the root of a Tree does not highlight descandants. May 2, 2023
@davep davep added bug Something isn't working and removed enhancement New feature or request labels May 2, 2023
@davep davep self-assigned this May 3, 2023
@davep davep changed the title Selecting the root of a Tree does not highlight descandants. Selecting the root of a Tree does not highlight descendants. May 3, 2023
@davep
Copy link
Contributor Author

davep commented May 3, 2023

While building up some test code for this, I think I've either uncovered three problems here, or three symptoms of an underlying problem. Using this code:

from textual.app          import App, ComposeResult
from textual.containers   import Horizontal
from textual.widgets      import Header, Footer, Tree
from textual.widgets.tree import TreeNode

class TreeLinesTestApp( App[ None ] ):

    CSS = """
    Tree {
        border: round red;
    }

    Tree:focus {
        border: double green;
    }
    """

    def compose( self ) -> ComposeResult:
        yield Header()
        with Horizontal():
            yield Tree("Root", id="root")
            yield Tree("Root", id="no-root")
        yield Footer()

    def populate( self, node: TreeNode, limit: int=3 ) -> Tree:
        for n in range( limit ):
            self.populate( node.add( str( n ) ), limit-1 )
        return node.tree

    def on_mount( self ) -> None:
        self.query_one( "#no-root", Tree ).show_root = False
        self.populate( self.query_one( "#root", Tree ).root ).root.expand_all()
        self.populate( self.query_one( "#no-root", Tree ).root ).root.expand_all()

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

I get this effect:

Screen.Recording.2023-05-03.at.14.15.37.mov

Note the following things:

  • The tree that is showing the root does highlight all of the lines when the root is selected
  • But... it doesn't do this by default, only if you move off the root and back onto it again
  • The tree that doesn't show the root never highlights from the top-level down, only from the second level.

davep added a commit to davep/textual-sandbox that referenced this issue May 3, 2023
@davep
Copy link
Contributor Author

davep commented May 3, 2023

Should also be noted that, within the bounds of what it means to hover, the same issue affects the style of lines and mouse hover too.

@davep
Copy link
Contributor Author

davep commented May 3, 2023

Possibly related, or at least related to one of the effects shown above: the shows-root Tree not showing a selected colour for all line when first focused might make some sense, in that selected in _render_line is predicated on self.has_focus. I'm still not 100% sure what would cause the lines to be drawn in regards to focus change (need to dig into that), but an attempt to force it to change with the addition of _on_focus that invalidates and redraws the tree seem to have no difference because, as best as I can tell, when _on_focus fires self.has_focus isn't yet true.

Need to keep this in mind as I dive deeper.

@davep
Copy link
Contributor Author

davep commented May 4, 2023

Just to clarify for myself here, as far as I can tell there are two distinct issues at play here:

  1. Tree doesn't handle showing lines as being selected when focus is first received.
  2. Tree doesn't properly show lines as being selected from the top level when show_root is False

It's been a bit too easy to be distracted by one while trying to reason about the other.

davep added a commit to davep/textual that referenced this issue May 4, 2023
Rather than always start at the root, the code should start at the beginning
of the path.

See Textualize#2397.
@github-actions
Copy link

github-actions bot commented May 4, 2023

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