From 48e5f5e0267d1019514c3c46263fd97d0cf6c27d Mon Sep 17 00:00:00 2001 From: Dave Pearson Date: Mon, 14 Nov 2022 12:22:24 +0000 Subject: [PATCH] Add Widget.move_child Adds a method to Widget that allows moving a child of that widget within its list of children. Options are to move before or after a specific location, or a sibling widget. Seeks to implement #1121. --- CHANGELOG.md | 1 + src/textual/widget.py | 73 ++++++++++++++++++++++ tests/test_widget_child_moving.py | 100 ++++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+) create mode 100644 tests/test_widget_child_moving.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 05165a611..a628871d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). the return value of `DOMQuery.remove`, which uses to return `self`. https://github.com/Textualize/textual/issues/1094 - Added Pilot.wait_for_animation +- Added `Widget.move_child` https://github.com/Textualize/textual/issues/1121 ### Changed diff --git a/src/textual/widget.py b/src/textual/widget.py index 9029eedcb..0f190dfff 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -480,6 +480,79 @@ class Widget(DOMNode): self.app._register(parent, *widgets, before=before, after=after) ) + def move_child( + self, + child: int | Widget, + before: int | Widget | None = None, + after: int | Widget | None = None, + ) -> None: + """Move a child widget within its parent's list of children. + + Args: + child (int | Widget): The child widget to move. + before: (int | Widget, optional): Optional location to move before. + after: (int | Widget, optional): Optional location to move after. + + Raises: + WidgetError: If there is a problem with the child or target. + """ + + # One or the other of before or after are required. Can't do + # neither, can't do both. + if before is None and after is None: + raise WidgetError("One of `before` or `after` is required.") + elif before is not None and after is not None: + raise WidgetError("Only one of `before`or `after` can be handled.") + + # Turn the child to move into a reference to the widget, doing some + # checks as we do so. + if isinstance(child, int): + try: + child = self.children[child] + except IndexError: + raise WidgetError( + f"An index of {child} for the child to move is out of bounds" + ) from None + else: + # We got an actual widget, so let's be sure it really is one of + # our children. + try: + _ = self.children.index(child) + except ValueError: + raise WidgetError(f"{child!r} is not a child of {self!r}") from None + + # Next, no matter if we're moving before or after, we just want to + # be sure that the target makes sense at all. So let's concentrate + # on that for a moment. + target = before if after is None else after + if isinstance(target, int): + try: + target = self.children[target] + except IndexError: + raise WidgetError( + f"An index of {target} for the target to move towards is out of bounds" + ) from None + elif isinstance(target, Widget): + # If we got given a widget from the off, let's be sure it's + # actually one of our children. + try: + _ = self.children.index(target) + except ValueError: + raise WidgetError(f"{target!r} is not a child of {self!r}") from None + + # At this point we should know what we're moving, and it should be a + # child; where we're moving it to, which should be within the child + # list; and how we're supposed to move it. All that's left is doing + # the right thing. + self.children._remove(child) + if before is not None: + self.children._insert(self.children.index(target), child) + else: + self.children._insert(self.children.index(target) + 1, child) + + # Request a refresh. + self.refresh(layout=True) + def compose(self) -> ComposeResult: """Called by Textual to create child widgets. diff --git a/tests/test_widget_child_moving.py b/tests/test_widget_child_moving.py new file mode 100644 index 000000000..43aa849eb --- /dev/null +++ b/tests/test_widget_child_moving.py @@ -0,0 +1,100 @@ +import pytest + +from textual.app import App +from textual.widget import Widget, WidgetError + +async def test_widget_move_child() -> None: + """Test moving a widget in a child list.""" + + # Test calling move_child with no direction. + async with App().run_test() as pilot: + child = Widget(Widget()) + await pilot.app.mount(child) + with pytest.raises(WidgetError): + pilot.app.screen.move_child(child) + + # Test calling move_child with more than one direction. + async with App().run_test() as pilot: + child = Widget(Widget()) + await pilot.app.mount(child) + with pytest.raises(WidgetError): + pilot.app.screen.move_child(child, before=1, after=2) + + # Test attempting to move a child that isn't ours. + async with App().run_test() as pilot: + child = Widget(Widget()) + await pilot.app.mount(child) + with pytest.raises(WidgetError): + pilot.app.screen.move_child(Widget(), before=child) + + # Test attempting to move relative to a widget that isn't a child. + async with App().run_test() as pilot: + child = Widget(Widget()) + await pilot.app.mount(child) + with pytest.raises(WidgetError): + pilot.app.screen.move_child(child, before=Widget()) + + # Make a background set of widgets. + widgets = [Widget(id=f"widget-{n}") for n in range( 10 )] + + # Test attempting to move past the end of the child list. + async with App().run_test() as pilot: + container = Widget(*widgets) + await pilot.app.mount(container) + with pytest.raises(WidgetError): + container.move_child(widgets[0], before=len(widgets)+10) + + # Test attempting to move before the end of the child list. + async with App().run_test() as pilot: + container = Widget(*widgets) + await pilot.app.mount(container) + with pytest.raises(WidgetError): + container.move_child(widgets[0], before=-(len(widgets)+10)) + + # Test the different permutations of moving one widget before another. + perms = ( + ( 1, 0 ), + ( widgets[1], 0 ), + ( 1, widgets[ 0 ] ), + ( widgets[ 1 ], widgets[ 0 ]) + ) + for child, target in perms: + async with App().run_test() as pilot: + container = Widget(*widgets) + await pilot.app.mount(container) + container.move_child(child, before=target) + assert container.children[0].id == "widget-1" + assert container.children[1].id == "widget-0" + assert container.children[2].id == "widget-2" + + # Test the different permutations of moving one widget after another. + perms = ( + ( 0, 1 ), + ( widgets[0], 1 ), + ( 0, widgets[ 1 ] ), + ( widgets[ 0 ], widgets[ 1 ]) + ) + for child, target in perms: + async with App().run_test() as pilot: + container = Widget(*widgets) + await pilot.app.mount(container) + container.move_child(child, after=target) + assert container.children[0].id == "widget-1" + assert container.children[1].id == "widget-0" + assert container.children[2].id == "widget-2" + + # Test moving after a child after the last child. + async with App().run_test() as pilot: + container = Widget(*widgets) + await pilot.app.mount(container) + container.move_child(widgets[0], after=widgets[-1]) + assert container.children[0].id == "widget-1" + assert container.children[-1].id == "widget-0" + + # Test moving after a child after the last child's numeric position. + async with App().run_test() as pilot: + container = Widget(*widgets) + await pilot.app.mount(container) + container.move_child(widgets[0], after=widgets[9]) + assert container.children[0].id == "widget-1" + assert container.children[-1].id == "widget-0"