Ensure hidden children aren't made visible if their parent is visible.

core/src/toga/style/applicator.py

@@ -25,7 +25,16 @@ def set_text_alignment(self, alignment):
     def set_hidden(self, hidden):
         self.widget._impl.set_hidden(hidden)
         for child in self.widget.children:
-            child.applicator.set_hidden(hidden)
+            # If the parent is hidden, then so are all children. However, if the
+            # parent is visible, then the child's explicit visibility style is
+            # taken into account. This visibility cascades into any
+            # grandchildren.
+            #
+            # parent hidden child hidden style child final hidden state
+            # ============= ================== ======================== True
+            #  True               True True          False              True
+            #  False         True               True False         False False
+            child.applicator.set_hidden(hidden or child.style._hidden)
 
     def set_font(self, font):
         # Changing the font of a widget can make the widget change size,

core/src/toga/style/pack.py

@@ -81,9 +81,14 @@ class IntrinsicSize(BaseIntrinsicSize):
 
     _depth = -1
 
-    def _debug(self, *args):
+    def _debug(self, *args):  # pragma: no cover
         print("    " * self.__class__._depth, *args)
 
+    @property
+    def _hidden(self):
+        "Does this style declaration define a object that should be hidden"
+        return self.visibility == HIDDEN
+
     def apply(self, prop, value):
         if self._applicator:
             if prop == "text_align":

core/tests/style/test_applicator.py

@@ -3,7 +3,7 @@
 import toga
 from toga.colors import REBECCAPURPLE
 from toga.fonts import FANTASY
-from toga.style.pack import RIGHT, VISIBLE
+from toga.style.pack import HIDDEN, RIGHT, VISIBLE
 from toga_dummy.utils import assert_action_performed_with
 
 
@@ -25,8 +25,16 @@ def __init__(self, *args, **kwargs):
 
 
 @pytest.fixture
-def child():
-    return TestLeafWidget(id="child_id")
+def grandchild():
+    return TestLeafWidget(id="grandchild_id")
+
+
+@pytest.fixture
+def child(grandchild):
+    child = TestWidget(id="child_id")
+    child.add(grandchild)
+
+    return child
 
 
 @pytest.fixture
@@ -44,25 +52,34 @@ def test_refresh(widget):
     assert_action_performed_with(widget, "refresh")
 
 
-def test_set_bounds(child, widget):
+def test_set_bounds(widget, child, grandchild):
     "Bounds changes are passed to all widgets in the tree"
     # Manually set location of the parent
-    widget.layout._origin_left = 10
-    widget.layout._origin_top = 20
-    widget.layout.content_width = 30
-    widget.layout.content_height = 40
+    widget.layout._origin_left = 100
+    widget.layout._origin_top = 200
+    widget.layout.content_width = 300
+    widget.layout.content_height = 400
 
     # Manually set location of the child
-    child.layout._origin_left = 1
-    child.layout._origin_top = 2
-    child.layout.content_width = 3
-    child.layout.content_height = 4
+    child.layout._origin_left = 10
+    child.layout._origin_top = 20
+    child.layout.content_width = 30
+    child.layout.content_height = 40
+
+    # Manually set location of the grandchild
+    grandchild.layout._origin_left = 1
+    grandchild.layout._origin_top = 2
+    grandchild.layout.content_width = 3
+    grandchild.layout.content_height = 4
 
     # Propegate the boundsq
     widget.applicator.set_bounds()
 
-    assert_action_performed_with(widget, "set bounds", x=10, y=20, width=30, height=40)
-    assert_action_performed_with(child, "set bounds", x=1, y=2, width=3, height=4)
+    assert_action_performed_with(
+        widget, "set bounds", x=100, y=200, width=300, height=400
+    )
+    assert_action_performed_with(child, "set bounds", x=10, y=20, width=30, height=40)
+    assert_action_performed_with(grandchild, "set bounds", x=1, y=2, width=3, height=4)
 
 
 def test_text_alignment(widget):
@@ -72,15 +89,50 @@ def test_text_alignment(widget):
     assert_action_performed_with(widget, "set alignment", alignment=RIGHT)
 
 
-def test_set_hidden(widget, child):
-    "Visibility can be set on a widget"
-    widget.applicator.set_hidden(True)
-
-    assert_action_performed_with(widget, "set hidden", hidden=True)
-    # The hide is applied transitively to the child
-    assert_action_performed_with(child, "set hidden", hidden=True)
-    # However, the style property of the child hasn't changed.
-    assert child.style.visibility == VISIBLE
+@pytest.mark.parametrize(
+    "child_visibility, grandchild_visibility, value, "
+    "widget_hidden, child_hidden, grandchild_hidden",
+    [
+        # Set widget hidden. All widgets are always hidden,
+        # no matter the actual style setting.
+        (VISIBLE, VISIBLE, True, True, True, True),
+        (VISIBLE, HIDDEN, True, True, True, True),
+        (HIDDEN, VISIBLE, True, True, True, True),
+        (HIDDEN, HIDDEN, True, True, True, True),
+        # Set widget visible. Visibility only cascades
+        # as far as the first HIDDEN widget
+        (VISIBLE, VISIBLE, False, False, False, False),
+        (VISIBLE, HIDDEN, False, False, False, True),
+        (HIDDEN, VISIBLE, False, False, True, True),
+        (HIDDEN, HIDDEN, False, False, True, True),
+    ],
+)
+def test_set_hidden(
+    widget,
+    child,
+    grandchild,
+    child_visibility,
+    grandchild_visibility,
+    value,
+    widget_hidden,
+    child_hidden,
+    grandchild_hidden,
+):
+    """Widget visibility can be controlled, and is transitive into children"""
+    # Set the explicit visibility of the child and grandchild
+    child.style.visibility = child_visibility
+    grandchild.style.visibility = grandchild_visibility
+
+    # Set widget visibility
+    widget.applicator.set_hidden(value)
+
+    assert_action_performed_with(widget, "set hidden", hidden=widget_hidden)
+    assert_action_performed_with(child, "set hidden", hidden=child_hidden)
+    assert_action_performed_with(grandchild, "set hidden", hidden=grandchild_hidden)
+
+    # The style property of the child and grandchild hasn't changed.
+    assert child.style.visibility == child_visibility
+    assert grandchild.style.visibility == grandchild_visibility
 
 
 def test_set_font(widget):

docs/reference/style/pack.rst

@@ -42,14 +42,15 @@ visible.
 
 **Initial value:** ``visible``
 
-Used to define whether the element should be drawn. A value of ``visible``
-means the element will be displayed. A value of ``none`` removes the element,
-but still allocates space for the element as if it were in the element tree.
-
-If an element with children is hidden, all it's children will be implicitly
-removed from view. However, if one of those children is to a parent that is
-*not* hidden, it will become visible again unless that widget has been
-*explicitly* marked as `hidden`.
+Used to define whether the element should be drawn. A value of ``visible`` means
+the element will be displayed. A value of ``hidden`` removes the element from
+view, but allocates space for the element as if it were still in the layout.
+
+Any children of a hidden element are implicitly removed from view.
+
+If a previously hidden element is made visible, any children of the element with
+a visibility of ``hidden`` will remain hidden. Any descendants of the hidden
+child will also remain hidden, regardless of their visibility.
 
 ``direction``
 -------------

testbed/tests/widgets/test_base.py

@@ -15,9 +15,13 @@ async def widget():
 
 async def test_visibility(widget, probe):
     "A widget (and it's children) can be made invisible"
-    child = toga.Button("Hello")
+    child = toga.Box(style=Pack(width=75, height=100, background_color=GREEN))
     child_probe = get_probe(child)
 
+    grandchild = toga.Button("Hello")
+    grandchild_probe = get_probe(grandchild)
+    child.add(grandchild)
+
     other = toga.Box(style=Pack(width=100, height=200, background_color=BLUE))
     other_probe = get_probe(other)
 
@@ -29,16 +33,18 @@ async def test_visibility(widget, probe):
     # Widgets are all visible an in place
     assert not probe.is_hidden
     assert not child_probe.is_hidden
+    assert not grandchild_probe.is_hidden
     probe.assert_layout(position=(0, 0), size=(100, 200))
     other_probe.assert_layout(position=(100, 0), size=(100, 200))
 
     # Hide the widget
     widget.style.visibility = HIDDEN
     await probe.redraw()
 
-    # Widget and child are no longer visible.
+    # Widgets are no longer visible.
     assert probe.is_hidden
     assert child_probe.is_hidden
+    assert grandchild_probe.is_hidden
     # Making the widget invisible doesn't affect layout
     probe.assert_layout(position=(0, 0), size=(100, 200))
     other_probe.assert_layout(position=(100, 0), size=(100, 200))
@@ -47,12 +53,50 @@ async def test_visibility(widget, probe):
     widget.style.visibility = VISIBLE
     await probe.redraw()
 
-    # Widgets and child are visible and in place again.
+    # Widgets are all visible and in place again.
     assert not probe.is_hidden
     assert not child_probe.is_hidden
+    assert not grandchild_probe.is_hidden
     probe.assert_layout(position=(0, 0), size=(100, 200))
     other_probe.assert_layout(position=(100, 0), size=(100, 200))
 
+    # Hide the widget again
+    widget.style.visibility = HIDDEN
+    await probe.redraw()
+
+    # Widgets are no longer visible.
+    assert probe.is_hidden
+    assert child_probe.is_hidden
+    assert grandchild_probe.is_hidden
+
+    # Mark the child style as visible.
+    child.style.visibility = VISIBLE
+    await probe.redraw()
+
+    # Root widget isn't visible, so neither descendent is visible.
+    assert probe.is_hidden
+    assert child_probe.is_hidden
+    assert grandchild_probe.is_hidden
+
+    # Explicitly mark the child style as hidden.
+    child.style.visibility = HIDDEN
+    await probe.redraw()
+
+    # Root widget isn't visible, so neither descendent is visible.
+    assert probe.is_hidden
+    assert child_probe.is_hidden
+    assert grandchild_probe.is_hidden
+
+    # Mark the root widget as visible again.
+    widget.style.visibility = VISIBLE
+    await probe.redraw()
+
+    # Root widget is visible again but the child is explicitly hidden,
+    # so it and the grandchild are still hidden
+    assert not probe.is_hidden
+    assert child_probe.is_hidden
+    assert grandchild_probe.is_hidden
+
 
 async def test_parenting(widget, probe):
     "A widget can be reparented between containers"