diff --git a/changes/3322.bugfix.rst b/changes/3322.bugfix.rst new file mode 100644 index 0000000000..8f3aeed279 --- /dev/null +++ b/changes/3322.bugfix.rst @@ -0,0 +1 @@ +``Pack.display="none"`` now correctly hides a widget, and removes it from the layout. diff --git a/core/src/toga/style/pack.py b/core/src/toga/style/pack.py index 536cf81044..732d58a91c 100644 --- a/core/src/toga/style/pack.py +++ b/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}") diff --git a/core/tests/style/pack/layout/test_display.py b/core/tests/style/pack/layout/test_display.py new file mode 100644 index 0000000000..e94f167371 --- /dev/null +++ b/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) diff --git a/core/tests/style/pack/test_apply.py b/core/tests/style/pack/test_apply.py index af1395555a..bc3651e329 100644 --- a/core/tests/style/pack/test_apply.py +++ b/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) diff --git a/docs/reference/style/pack.rst b/docs/reference/style/pack.rst index 62af4b44ec..073a9eb826 100644 --- a/docs/reference/style/pack.rst +++ b/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`` --------------