Exclude display=NONE nodes from layout

changes/3322.bugfix.rst

@@ -0,0 +1 @@
+``Pack.display="none"`` now correctly hides a widget, and removes it from the layout.

core/src/toga/style/pack.py

@@ -268,7 +268,7 @@ def _debug(cls, *args: str) -> None:  # pragma: no cover
     @property
     def _hidden(self) -> bool:
         """Does this style declaration define an object that should be hidden."""
-        return self.visibility == HIDDEN
+        return self.visibility == HIDDEN or self.display == NONE
 
     def _apply(self, names: set) -> None:
         if "text_align" in names:
@@ -287,19 +287,19 @@ def _apply(self, names: set) -> None:
             self._applicator.set_color(self.color)
         if "background_color" in names:
             self._applicator.set_background_color(self.background_color)
-        if "visibility" in names:
-            value = self.visibility
-            if value == VISIBLE:
-                # If visibility is being set to VISIBLE, look up the chain to
-                # see if an ancestor is hidden.
+        if names & {"visibility", "display"}:
+            value = self._hidden
+
+            if not value:
+                # If this style is visible, look up the chain to see if an ancestor is
+                # hidden
                 widget = self._applicator.widget
                 while widget := widget.parent:
                     if widget.style._hidden:
-                        value = HIDDEN
+                        value = True
                         break
 
-            self._applicator.set_hidden(value == HIDDEN)
-
+            self._applicator.set_hidden(value)
         if names & {
             "font_family",
             "font_size",
@@ -420,7 +420,9 @@ def _layout_node(
                 # self._debug(f"AUTO {available_height=}")
                 min_height = 0
 
-        if node.children:
+        if node.children and any(
+            child.style.display != NONE for child in node.children
+        ):
             min_width, width, min_height, height = self._layout_children(
                 available_width=available_width,
                 available_height=available_height,
@@ -514,7 +516,9 @@ def _layout_children(
         # intrinsic non-flexible dimension. While iterating, collect the flex
         # total of remaining elements.
 
-        for i, child in enumerate(node.children):
+        children = [child for child in node.children if not child.style.display == NONE]
+
+        for i, child in enumerate(children):
             # self._debug(f"PASS 1 {child}")
             if child.style[main_name] != NONE:
                 # self._debug(f"- fixed {main_name} {child.style[main_name]}")
@@ -641,7 +645,7 @@ def _layout_children(
             # the flex calculation.
 
             # self._debug(f"PASS 1a; {quantum=}")
-            for child in node.children:
+            for child in children:
                 child_intrinsic_main = getattr(child.intrinsic, main_name)
                 if child.style.flex and child_intrinsic_main is not None:
                     try:
@@ -669,7 +673,7 @@ def _layout_children(
 
         # Pass 2: Lay out children with an intrinsic flexible main-axis size, or no
         # main-axis size specification at all.
-        for child in node.children:
+        for child in children:
             # self._debug(f"PASS 2 {child}")
             if child.style[main_name] != NONE:
                 # self._debug(f"- already laid out (explicit {main_name})")
@@ -787,7 +791,7 @@ def _layout_children(
         cross = 0
         min_cross = 0
 
-        for child in node.children:
+        for child in children:
             # self._debug(f"PASS 3: {child} AT MAIN-AXIS OFFSET {offset}")
             if main_start == RIGHT:
                 # Needs special casing, since it's still ultimately content_left that
@@ -843,7 +847,7 @@ def _layout_children(
             effective_cross_start = cross_start
             effective_cross_end = cross_end
 
-        for child in node.children:
+        for child in children:
             # self._debug(f"PASS 4: {child}")
             extra = cross - (
                 getattr(child.layout, f"content_{cross_name}")

core/tests/style/pack/layout/test_display.py

@@ -0,0 +1,88 @@
+from toga.style.pack import COLUMN, NONE, PACK, Pack
+
+from ..utils import ExampleNode, ExampleViewport, assert_layout
+
+
+def test_display_none():
+    """Setting a node to display=NONE removes it from the layout."""
+    root = ExampleNode(
+        "app",
+        style=Pack(direction=COLUMN),
+        children=[
+            ExampleNode(
+                "first",
+                style=Pack(width=100, height=110),
+            ),
+            rabbit := ExampleNode(
+                "second",
+                style=Pack(width=100, height=120),
+            ),
+            ExampleNode(
+                "third",
+                style=Pack(width=100, height=130),
+            ),
+        ],
+    )
+
+    root.style.layout(ExampleViewport(640, 480))
+    initial = (
+        root,
+        (100, 360),
+        (640, 480),
+        {
+            "origin": (0, 0),
+            "content": (640, 480),
+            "children": [
+                {
+                    "origin": (0, 0),
+                    "content": (100, 110),
+                },
+                {
+                    "origin": (0, 110),
+                    "content": (100, 120),
+                },
+                {
+                    "origin": (0, 230),
+                    "content": (100, 130),
+                },
+            ],
+        },
+    )
+    assert_layout(*initial)
+
+    # Turn off display for the middle child.
+    rabbit.style.display = NONE
+
+    root.style.layout(ExampleViewport(640, 480))
+    assert_layout(
+        root,
+        (100, 240),
+        (640, 480),
+        {
+            "origin": (0, 0),
+            "content": (640, 480),
+            "children": [
+                {
+                    "origin": (0, 0),
+                    "content": (100, 110),
+                },
+                # As far as the vanished node knows, its values haven't changed, because
+                # its layout hasn't been refreshed.
+                {
+                    "origin": (0, 110),
+                    "content": (100, 120),
+                },
+                # But the third child has slid upward to take its place.
+                {
+                    "origin": (0, 110),
+                    "content": (100, 130),
+                },
+            ],
+        },
+    )
+
+    rabbit.style.display = PACK
+
+    # Everything's back to normal.
+    root.style.layout(ExampleViewport(640, 480))
+    assert_layout(*initial)

core/tests/style/pack/test_apply.py

@@ -1,5 +1,7 @@
 from unittest.mock import call
 
+import pytest
+
 from toga.colors import rgb
 from toga.fonts import Font
 from toga.style.pack import (
@@ -10,6 +12,7 @@
     ITALIC,
     LEFT,
     NONE,
+    PACK,
     RIGHT,
     RTL,
     SMALL_CAPS,
@@ -106,14 +109,28 @@ def test_set_multiple_layout_properties():
     root.refresh.assert_called_once_with()
 
 
-def test_set_visibility_hidden():
-    root = ExampleNode("app", style=Pack(visibility=HIDDEN))
+@pytest.mark.parametrize(
+    "name, value",
+    [
+        ("visibility", HIDDEN),
+        ("display", NONE),
+    ],
+)
+def test_set_visibility_hidden(name, value):
+    root = ExampleNode("app", style=Pack(**{name: value}))
     root.style.apply()
     root._impl.set_hidden.assert_called_once_with(True)
 
 
-def test_set_visibility_inherited():
-    """Nodes should be hidden when an ancestor is hidden."""
+@pytest.mark.parametrize(
+    "name, on, off",
+    [
+        ("visibility", VISIBLE, HIDDEN),
+        ("display", PACK, NONE),
+    ],
+)
+def test_set_visibility_inherited(name, on, off):
+    """Nodes should be hidden when an ancestor is hidden via display or visibility."""
     grandparent = ExampleParentNode("grandparent", style=Pack())
     parent = ExampleParentNode("parent", style=Pack())
     child = ExampleNode("child", style=Pack())
@@ -134,24 +151,24 @@ def assert_hidden_called(grandparent_value, parent_value, child_value):
             node._impl.set_hidden.reset_mock()
 
     # Hiding grandparent should hide all.
-    grandparent.style.visibility = HIDDEN
+    grandparent.style[name] = off
     assert_hidden_called(True, True, True)
 
     # Just setting child or parent to VISIBLE won't trigger an apply, because that's
     # their default value. So first, set them to hidden.
-    parent.style.visibility = HIDDEN
+    parent.style[name] = off
     assert_hidden_called(None, True, True)
 
-    child.style.visibility = HIDDEN
+    child.style[name] = off
     assert_hidden_called(None, None, True)
 
     # Then set them to visible. They should still not actually be shown.
-    parent.style.visibility = VISIBLE
+    parent.style[name] = on
     assert_hidden_called(None, True, True)
 
-    child.style.visibility = VISIBLE
+    child.style[name] = on
     assert_hidden_called(None, None, True)
 
     # Show grandparent again; the other two should reappear.
-    grandparent.style.visibility = VISIBLE
+    grandparent.style[name] = on
     assert_hidden_called(False, False, False)

docs/reference/style/pack.rst

@@ -31,9 +31,8 @@ Pack style properties
 
 Used to define how to display the widget. A value of ``pack`` will apply
 the pack layout algorithm to this node and its descendants. A value of
-``none`` removes the widget from the layout entirely. Space will be allocated
-for the widget as if it were there, but the widget itself will not be
-visible.
+``none`` removes the widget from the layout entirely; it won't be shown,
+and it won't occupy space.
 
 ``visibility``
 --------------