From 5136714f7d276f019b6d28541a47fae2af68896c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 8 Aug 2023 13:48:25 +0100
Subject: [PATCH 01/11] Add regression tests for #3053

---
 tests/test_focus.py | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tests/test_focus.py b/tests/test_focus.py
index a03b9b53cd..4045ccb250 100644
--- a/tests/test_focus.py
+++ b/tests/test_focus.py
@@ -1,8 +1,10 @@
 import pytest
 
 from textual.app import App
+from textual.containers import Container
 from textual.screen import Screen
 from textual.widget import Widget
+from textual.widgets import Button
 
 
 class Focusable(Widget, can_focus=True):
@@ -201,3 +203,41 @@ def test_focus_next_and_previous_with_str_selector_without_self(screen: Screen):
     assert screen.focus_previous(".a").id == "foo"
     assert screen.focus_previous(".a").id == "foo"
     assert screen.focus_previous(".b").id == "baz"
+
+
+async def test_focus_does_not_move_to_invisible_widgets():
+    """Make sure invisible widgets don't get focused by accident.
+
+    This is kind of a regression test for https://github.com/Textualize/textual/issues/3053,
+    but not really.
+    """
+
+    class MyApp(App):
+        CSS = "#inv { visibility: hidden; }"
+
+        def compose(self):
+            yield Button("one", id="one")
+            yield Button("two", id="inv")
+            yield Button("three", id="three")
+
+    app = MyApp()
+    async with app.run_test():
+        assert app.focused.id == "one"
+        assert app.screen.focus_next().id == "three"
+
+
+async def test_focus_moves_to_visible_widgets_inside_invisible_containers():
+    """Regression test for https://github.com/Textualize/textual/issues/3053."""
+
+    class MyApp(App):
+        CSS = "#inv { visibility: hidden; }"
+
+        def compose(self):
+            yield Button(id="one")
+            with Container(id="inv"):
+                yield Button(id="three")
+
+    app = MyApp()
+    async with app.run_test():
+        assert app.focused.id == "one"
+        assert app.screen.focus_next().id == "three"

From 70a995f6e1075debd84a4791e19d5a667badd4d7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 8 Aug 2023 13:49:31 +0100
Subject: [PATCH 02/11] 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.
---
 CHANGELOG.md          |  1 +
 src/textual/screen.py | 11 +++++++++--
 src/textual/widget.py | 12 +-----------
 3 files changed, 11 insertions(+), 13 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 3a1770c00b..189dd7f970 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed unintuitive sizing behaviour of TabbedContent https://github.com/Textualize/textual/issues/2411
 - Fixed relative units not always expanding auto containers https://github.com/Textualize/textual/pull/3059
 - Fixed background refresh https://github.com/Textualize/textual/issues/3055
+- Fixed issue with visible children inside invisible container when moving focus https://github.com/Textualize/textual/issues/3053
 
 
 ## [0.32.0] - 2023-08-03
diff --git a/src/textual/screen.py b/src/textual/screen.py
index 1a9b9af2ee..dcb6500612 100644
--- a/src/textual/screen.py
+++ b/src/textual/screen.py
@@ -6,6 +6,7 @@
 from __future__ import annotations
 
 from functools import partial
+from operator import attrgetter
 from typing import (
     TYPE_CHECKING,
     Awaitable,
@@ -293,7 +294,10 @@ def focus_chain(self) -> list[Widget]:
 
         widgets: list[Widget] = []
         add_widget = widgets.append
-        stack: list[Iterator[Widget]] = [iter(self.focusable_children)]
+        focus_sorter = attrgetter("_focus_sort_key")
+        stack: list[Iterator[Widget]] = [
+            iter(sorted(self.displayed_children, key=focus_sorter))
+        ]
         pop = stack.pop
         push = stack.append
 
@@ -303,7 +307,10 @@ def focus_chain(self) -> list[Widget]:
                 pop()
             else:
                 if node.is_container and node.can_focus_children:
-                    push(iter(node.focusable_children))
+                    sorted_displayed_children = sorted(
+                        node.displayed_children, key=focus_sorter
+                    )
+                    push(iter(sorted_displayed_children))
                 if node.focusable:
                     add_widget(node)
 
diff --git a/src/textual/widget.py b/src/textual/widget.py
index 6d0673ee4d..456d00ee30 100644
--- a/src/textual/widget.py
+++ b/src/textual/widget.py
@@ -1498,17 +1498,7 @@ def _self_or_ancestors_disabled(self) -> bool:
     @property
     def focusable(self) -> bool:
         """Can this widget currently be focused?"""
-        return self.can_focus and not self._self_or_ancestors_disabled
-
-    @property
-    def focusable_children(self) -> list[Widget]:
-        """Get the children which may be focused.
-
-        Returns:
-            List of widgets that can receive focus.
-        """
-        focusable = [child for child in self._nodes if child.display and child.visible]
-        return sorted(focusable, key=attrgetter("_focus_sort_key"))
+        return self.can_focus and self.visible and not self._self_or_ancestors_disabled
 
     @property
     def _focus_sort_key(self) -> tuple[int, int]:

From 70ff85eed39af72ebc856ace90415982cdd8aa34 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 8 Aug 2023 14:55:40 +0100
Subject: [PATCH 03/11] Make note of removed property.

---
 CHANGELOG.md | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 189dd7f970..ce5263c83f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed issue with visible children inside invisible container when moving focus https://github.com/Textualize/textual/issues/3053
 
 
+### Removed
+
+- Property `Widget.focusable_children` https://github.com/Textualize/textual/pull/3070
+
+
 ## [0.32.0] - 2023-08-03
 
 ### Added

From 2818817cffefb5bc05c0629b677739e6a7852df2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 8 Aug 2023 15:50:28 +0100
Subject: [PATCH 04/11] Add regression test for #3071.

---
 tests/test_visibility_change.py | 43 ------------------
 tests/test_visible.py           | 78 +++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+), 43 deletions(-)
 delete mode 100644 tests/test_visibility_change.py
 create mode 100644 tests/test_visible.py

diff --git a/tests/test_visibility_change.py b/tests/test_visibility_change.py
deleted file mode 100644
index 7006827ee2..0000000000
--- a/tests/test_visibility_change.py
+++ /dev/null
@@ -1,43 +0,0 @@
-"""See https://github.com/Textualize/textual/issues/1355 as the motivation for these tests."""
-
-from textual.app import App, ComposeResult
-from textual.containers import VerticalScroll
-from textual.widget import Widget
-
-
-class VisibleTester(App[None]):
-    """An app for testing visibility changes."""
-
-    CSS = """
-    Widget {
-        height: 1fr;
-    }
-    .hidden {
-        visibility: hidden;
-    }
-    """
-
-    def compose(self) -> ComposeResult:
-        yield VerticalScroll(
-            Widget(id="keep"), Widget(id="hide-via-code"), Widget(id="hide-via-css")
-        )
-
-
-async def test_visibility_changes() -> None:
-    """Test changing visibility via code and CSS."""
-    async with VisibleTester().run_test() as pilot:
-        assert pilot.app.query_one("#keep").visible is True
-        assert pilot.app.query_one("#hide-via-code").visible is True
-        assert pilot.app.query_one("#hide-via-css").visible is True
-
-        pilot.app.query_one("#hide-via-code").styles.visibility = "hidden"
-        await pilot.pause(0)
-        assert pilot.app.query_one("#keep").visible is True
-        assert pilot.app.query_one("#hide-via-code").visible is False
-        assert pilot.app.query_one("#hide-via-css").visible is True
-
-        pilot.app.query_one("#hide-via-css").set_class(True, "hidden")
-        await pilot.pause(0)
-        assert pilot.app.query_one("#keep").visible is True
-        assert pilot.app.query_one("#hide-via-code").visible is False
-        assert pilot.app.query_one("#hide-via-css").visible is False
diff --git a/tests/test_visible.py b/tests/test_visible.py
new file mode 100644
index 0000000000..3d991d8588
--- /dev/null
+++ b/tests/test_visible.py
@@ -0,0 +1,78 @@
+from textual.app import App, ComposeResult
+from textual.containers import VerticalScroll
+from textual.widget import Widget
+
+
+async def test_visibility_changes() -> None:
+    """Test changing visibility via code and CSS.
+
+    See https://github.com/Textualize/textual/issues/1355 as the motivation for these tests.
+    """
+
+    class VisibleTester(App[None]):
+        """An app for testing visibility changes."""
+
+        CSS = """
+        Widget {
+            height: 1fr;
+        }
+        .hidden {
+            visibility: hidden;
+        }
+        """
+
+        def compose(self) -> ComposeResult:
+            yield VerticalScroll(
+                Widget(id="keep"), Widget(id="hide-via-code"), Widget(id="hide-via-css")
+            )
+
+    async with VisibleTester().run_test() as pilot:
+        assert pilot.app.query_one("#keep").visible is True
+        assert pilot.app.query_one("#hide-via-code").visible is True
+        assert pilot.app.query_one("#hide-via-css").visible is True
+
+        pilot.app.query_one("#hide-via-code").styles.visibility = "hidden"
+        await pilot.pause(0)
+        assert pilot.app.query_one("#keep").visible is True
+        assert pilot.app.query_one("#hide-via-code").visible is False
+        assert pilot.app.query_one("#hide-via-css").visible is True
+
+        pilot.app.query_one("#hide-via-css").set_class(True, "hidden")
+        await pilot.pause(0)
+        assert pilot.app.query_one("#keep").visible is True
+        assert pilot.app.query_one("#hide-via-code").visible is False
+        assert pilot.app.query_one("#hide-via-css").visible is False
+
+
+async def test_visible_is_inherited() -> None:
+    """Regression test for https://github.com/Textualize/textual/issues/3071"""
+
+    class InheritedVisibilityApp(App[None]):
+        CSS = """
+        #four {
+            visibility: visible;
+        }
+
+        #six {
+            visibility: hidden;
+        }
+        """
+
+        def compose(self):
+            yield Widget(id="one")
+            with VerticalScroll(id="two"):
+                yield Widget(id="three")
+                with VerticalScroll(id="four"):
+                    yield Widget(id="five")
+                    with VerticalScroll(id="six"):
+                        yield Widget(id="seven")
+
+    app = InheritedVisibilityApp()
+    async with app.run_test():
+        assert app.query_one("#one").visible
+        assert app.query_one("#two").visible
+        assert app.query_one("#three").visible
+        assert app.query_one("#four").visible
+        assert app.query_one("#five").visible
+        assert not app.query_one("#six").visible
+        assert not app.query_one("#seven").visible

From 590668627e20e2299869b37679a1814f3fb8abd8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 8 Aug 2023 15:56:01 +0100
Subject: [PATCH 05/11] Fix #3071.

---
 CHANGELOG.md       |  5 +++++
 src/textual/dom.py | 16 ++++++++++++----
 2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index ce5263c83f..cae3f3f705 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 - Fixed issue with visible children inside invisible container when moving focus https://github.com/Textualize/textual/issues/3053
 
 
+### Changed
+
+- Breaking change: `DOMNode.visible` now takes into account full DOM to report whether a node is visible or not.
+
+
 ### Removed
 
 - Property `Widget.focusable_children` https://github.com/Textualize/textual/pull/3070
diff --git a/src/textual/dom.py b/src/textual/dom.py
index 3cc5051728..a65b8beeea 100644
--- a/src/textual/dom.py
+++ b/src/textual/dom.py
@@ -612,13 +612,21 @@ def display(self, new_val: bool | str) -> None:
 
     @property
     def visible(self) -> bool:
-        """Is the visibility style set to a visible state?
+        """Is this widget visible in the DOM?
 
-        May be set to a boolean to make the node visible (`True`) or invisible (`False`), or to any valid value for the `visibility` rule.
+        If a widget hasn't had its visibility set explicitly, then it inherits it from its
+        DOM ancestors.
 
-        When a node is invisible, Textual will reserve space for it, but won't display anything there.
+        This may be set explicitly to override inherited values.
+        The valid values include the valid values for the `visibility` rule and the booleans
+        `True` or `False`, to set the widget to be visible or invisible, respectively.
+
+        When a node is invisible, Textual will reserve space for it, but won't display anything.
         """
-        return self.styles.visibility != "hidden"
+        own_value = self.styles.get_rule("visibility")
+        if own_value is not None:
+            return own_value != "hidden"
+        return self.parent.visible if self.parent else True
 
     @visible.setter
     def visible(self, new_value: bool | str) -> None:

From 29d6b2c92dd701d2a59be159d82e06a479e2c781 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 8 Aug 2023 15:56:20 +0100
Subject: [PATCH 06/11] Fix regression test for #3053.

---
 tests/test_focus.py | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/tests/test_focus.py b/tests/test_focus.py
index 4045ccb250..5011142dcd 100644
--- a/tests/test_focus.py
+++ b/tests/test_focus.py
@@ -230,7 +230,10 @@ async def test_focus_moves_to_visible_widgets_inside_invisible_containers():
     """Regression test for https://github.com/Textualize/textual/issues/3053."""
 
     class MyApp(App):
-        CSS = "#inv { visibility: hidden; }"
+        CSS = """
+        #inv { visibility: hidden; }
+        #three { visibility: visible; }
+        """
 
         def compose(self):
             yield Button(id="one")

From 242ffc1366185f9a50f7f7876263f270ddef19d8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 8 Aug 2023 16:54:12 +0100
Subject: [PATCH 07/11] 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: https://github.com/Textualize/textual/pull/3070#issuecomment-1669683285
---
 src/textual/screen.py | 44 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 8 deletions(-)

diff --git a/src/textual/screen.py b/src/textual/screen.py
index dcb6500612..6704b14551 100644
--- a/src/textual/screen.py
+++ b/src/textual/screen.py
@@ -295,23 +295,51 @@ def focus_chain(self) -> list[Widget]:
         widgets: list[Widget] = []
         add_widget = widgets.append
         focus_sorter = attrgetter("_focus_sort_key")
-        stack: list[Iterator[Widget]] = [
+        node_stack: list[Iterator[Widget]] = [
             iter(sorted(self.displayed_children, key=focus_sorter))
         ]
-        pop = stack.pop
-        push = stack.append
+        pop_node = node_stack.pop
+        push_node = node_stack.append
 
-        while stack:
-            node = next(stack[-1], None)
+        # Instead of relying on the property `DOMNode.visible`, we manually keep track
+        # of the visibility of the DOM to save on DOM traversals inside `.visible`.
+        visibility_stack: list[bool] = [self.visible]
+        pop_visibility = visibility_stack.pop
+        push_visibility = visibility_stack.append
+
+        while node_stack:
+            node = next(node_stack[-1], None)
             if node is None:
-                pop()
+                pop_node()
+                pop_visibility()
             else:
+                node_visibility = node.styles.get_rule("visibility")
                 if node.is_container and node.can_focus_children:
                     sorted_displayed_children = sorted(
                         node.displayed_children, key=focus_sorter
                     )
-                    push(iter(sorted_displayed_children))
-                if node.focusable:
+                    push_node(iter(sorted_displayed_children))
+                    # When we push a new container to the stack, we need to update
+                    # the visibility stack.
+                    if node_visibility is None:
+                        # If the node doesn't have explicit visibility set, we inherit
+                        # it from the stack, unless we're at the top of the stack,
+                        # in which case we must compute the node visibility.
+                        push_visibility(
+                            visibility_stack[-1] if visibility_stack else node.visible
+                        )
+                    else:
+                        push_visibility(node_visibility != "hidden")
+                node_is_visible = (
+                    node_visibility != "hidden"
+                    if node_visibility
+                    else visibility_stack[-1]
+                )
+                if (  # Same as `if node.focusable`, but we cache inherited visibility.
+                    node_is_visible
+                    and node.can_focus
+                    and not node._self_or_ancestors_disabled
+                ):
                     add_widget(node)
 
         return widgets

From febbd6153229b63deca5ba698095e39aa562d3c6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 8 Aug 2023 17:12:28 +0100
Subject: [PATCH 08/11] Make test more robust.

---
 tests/test_focus.py | 66 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 66 insertions(+)

diff --git a/tests/test_focus.py b/tests/test_focus.py
index 5011142dcd..152213c19d 100644
--- a/tests/test_focus.py
+++ b/tests/test_focus.py
@@ -244,3 +244,69 @@ def compose(self):
     async with app.run_test():
         assert app.focused.id == "one"
         assert app.screen.focus_next().id == "three"
+
+
+async def test_focus_chain_handles_inherited_visibility():
+    """Regression test for https://github.com/Textualize/textual/issues/3053
+
+    This is more or less a test for the interactions between #3053 and #3071.
+    We want to make sure that the focus chain is computed correctly when going through
+    a DOM with containers with all sorts of visibilities set.
+    """
+
+    class W(Widget):
+        can_focus = True
+
+    w1 = W(id="one")
+    c2 = Container(id="two")
+    w3 = W(id="three")
+    c4 = Container(id="four")
+    w5 = W(id="five")
+    c6 = Container(id="six")
+    w7 = W(id="seven")
+    c8 = Container(id="eight")
+    w9 = W(id="nine")
+    w10 = W(id="ten")
+    w11 = W(id="eleven")
+    w12 = W(id="twelve")
+    w13 = W(id="thirteen")
+
+    class InheritedVisibilityApp(App[None]):
+        CSS = """
+        #four, #eight, #ten {
+            visibility: visible;
+        }
+
+        #six {
+            visibility: hidden;
+        }
+        """
+
+        def compose(self):
+            yield w1  # visible, inherited
+            with c2:  # visible, inherited
+                yield w3  # visible, inherited
+                with c4:  # visible, set
+                    yield w5  # visible, inherited
+                    with c6:  # hidden, set
+                        yield w7  # hidden, inherited
+                        with c8:  # visible, set
+                            yield w9  # visible, inherited
+                        yield w10  # visible, set
+                    yield w11  # visible, inherited
+                yield w12  # visible, inherited
+            yield w13  # visible, inherited
+
+    app = InheritedVisibilityApp()
+    async with app.run_test():
+        focus_chain = app.screen.focus_chain
+        assert focus_chain == [
+            w1,
+            w3,
+            w5,
+            w9,
+            w10,
+            w11,
+            w12,
+            w13,
+        ]

From e9c6d5f96a25943501006a1736648553332c2c7a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 8 Aug 2023 17:14:25 +0100
Subject: [PATCH 09/11] Make test more robust.

---
 tests/test_focus.py | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/tests/test_focus.py b/tests/test_focus.py
index 152213c19d..489d808c26 100644
--- a/tests/test_focus.py
+++ b/tests/test_focus.py
@@ -277,7 +277,7 @@ class InheritedVisibilityApp(App[None]):
             visibility: visible;
         }
 
-        #six {
+        #six, #thirteen {
             visibility: hidden;
         }
         """
@@ -295,7 +295,7 @@ def compose(self):
                         yield w10  # visible, set
                     yield w11  # visible, inherited
                 yield w12  # visible, inherited
-            yield w13  # visible, inherited
+            yield w13  # invisible, set
 
     app = InheritedVisibilityApp()
     async with app.run_test():
@@ -308,5 +308,4 @@ def compose(self):
             w10,
             w11,
             w12,
-            w13,
         ]

From ae773edef213673def9cd0ca616ee133554ce255 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Mon, 21 Aug 2023 17:28:00 +0100
Subject: [PATCH 10/11] 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
---
 src/textual/screen.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/textual/screen.py b/src/textual/screen.py
index b0a2de5f76..203dcb2262 100644
--- a/src/textual/screen.py
+++ b/src/textual/screen.py
@@ -314,6 +314,8 @@ def focus_chain(self) -> list[Widget]:
                 pop_visibility()
             else:
                 node_visibility = node.styles.get_rule("visibility")
+                if node.disabled:
+                    continue
                 if node.is_container and node.can_focus_children:
                     sorted_displayed_children = sorted(
                         node.displayed_children, key=focus_sorter
@@ -335,11 +337,9 @@ def focus_chain(self) -> list[Widget]:
                     if node_visibility
                     else visibility_stack[-1]
                 )
-                if (  # Same as `if node.focusable`, but we cache inherited visibility.
-                    node_is_visible
-                    and node.can_focus
-                    and not node._self_or_ancestors_disabled
-                ):
+                # Same check as `if node.focusable`, but we cached inherited visibility
+                # and we also skipped disabled nodes altogether.
+                if node_is_visible and node.can_focus:
                     add_widget(node)
 
         return widgets

From f8b270db5570b2fc7435915bbc0eec9e5aaed388 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Rodrigo=20Gir=C3=A3o=20Serr=C3=A3o?=
 <5621605+rodrigogiraoserrao@users.noreply.github.com>
Date: Tue, 22 Aug 2023 14:23:05 +0100
Subject: [PATCH 11/11] 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: https://github.com/Textualize/textual/pull/3070#pullrequestreview-1587295458
---
 src/textual/screen.py | 52 +++++++++++++++++--------------------------
 1 file changed, 21 insertions(+), 31 deletions(-)

diff --git a/src/textual/screen.py b/src/textual/screen.py
index 203dcb2262..15efc9faf2 100644
--- a/src/textual/screen.py
+++ b/src/textual/screen.py
@@ -295,48 +295,38 @@ def focus_chain(self) -> list[Widget]:
         widgets: list[Widget] = []
         add_widget = widgets.append
         focus_sorter = attrgetter("_focus_sort_key")
-        node_stack: list[Iterator[Widget]] = [
-            iter(sorted(self.displayed_children, key=focus_sorter))
+        # We traverse the DOM and keep track of where we are at with a node stack.
+        # Additionally, we manually keep track of the visibility of the DOM
+        # instead of relying on the property `.visible` to save on DOM traversals.
+        # node_stack: list[tuple[iterator over node children, node visibility]]
+        node_stack: list[tuple[Iterator[Widget], bool]] = [
+            (
+                iter(sorted(self.displayed_children, key=focus_sorter)),
+                self.visible,
+            )
         ]
-        pop_node = node_stack.pop
-        push_node = node_stack.append
-
-        # Instead of relying on the property `DOMNode.visible`, we manually keep track
-        # of the visibility of the DOM to save on DOM traversals inside `.visible`.
-        visibility_stack: list[bool] = [self.visible]
-        pop_visibility = visibility_stack.pop
-        push_visibility = visibility_stack.append
+        pop = node_stack.pop
+        push = node_stack.append
 
         while node_stack:
-            node = next(node_stack[-1], None)
+            children_iterator, parent_visibility = node_stack[-1]
+            node = next(children_iterator, None)
             if node is None:
-                pop_node()
-                pop_visibility()
+                pop()
             else:
-                node_visibility = node.styles.get_rule("visibility")
                 if node.disabled:
                     continue
+                node_styles_visibility = node.styles.get_rule("visibility")
+                node_is_visible = (
+                    node_styles_visibility != "hidden"
+                    if node_styles_visibility
+                    else parent_visibility  # Inherit visibility if the style is unset.
+                )
                 if node.is_container and node.can_focus_children:
                     sorted_displayed_children = sorted(
                         node.displayed_children, key=focus_sorter
                     )
-                    push_node(iter(sorted_displayed_children))
-                    # When we push a new container to the stack, we need to update
-                    # the visibility stack.
-                    if node_visibility is None:
-                        # If the node doesn't have explicit visibility set, we inherit
-                        # it from the stack, unless we're at the top of the stack,
-                        # in which case we must compute the node visibility.
-                        push_visibility(
-                            visibility_stack[-1] if visibility_stack else node.visible
-                        )
-                    else:
-                        push_visibility(node_visibility != "hidden")
-                node_is_visible = (
-                    node_visibility != "hidden"
-                    if node_visibility
-                    else visibility_stack[-1]
-                )
+                    push((iter(sorted_displayed_children), node_is_visible))
                 # Same check as `if node.focusable`, but we cached inherited visibility
                 # and we also skipped disabled nodes altogether.
                 if node_is_visible and node.can_focus: