Skip to content

Commit

Permalink
Change TreeNode.toggle to act only from the source node
Browse files Browse the repository at this point in the history
See #1644 (comment)
where Darren raises the excellent point that while the "technically correct"
approach that I had was... well, technically correct I guess (it toggled all
the nodes from the target node down), it didn't have what was likely the
desired effect.

So this commit does away with the previous logic for doing the toggle and
instead simply calls on expand or collapse depending on the state of the
source node.
  • Loading branch information
davep committed Jan 24, 2023
1 parent 6743ec6 commit a365836
Showing 1 changed file with 4 additions and 14 deletions.
18 changes: 4 additions & 14 deletions src/textual/widgets/_tree.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,26 +201,16 @@ def collapse(self, *, collapse_all: bool = False) -> None:
self._collapse(collapse_all)
self._tree._invalidate()

def _toggle(self, toggle_all: bool) -> None:
"""Toggle the expanded state of the node.
Args:
toggle_all: If `True` toggle all offspring at all depths.
"""
self._expanded = not self._expanded
if toggle_all:
for child in self.children:
child._toggle(toggle_all)
self._updates += 1

def toggle(self, *, toggle_all: bool = False) -> None:
"""Toggle the expanded state.
Args:
toggle_all: If `True` toggle all offspring at all depths.
"""
self._toggle(toggle_all)
self._tree._invalidate()
if self._expanded:

This comment has been minimized.

Copy link
@darrenburns

darrenburns Jan 24, 2023

Member

I think this makes more sense to me, but the docstring should probably be updated too.

self.collapse(collapse_all=toggle_all)
else:
self.expand(expand_all=toggle_all)

@property
def label(self) -> TextType:
Expand Down

0 comments on commit a365836

Please sign in to comment.