fix(listview): update index after items removed (#5135)

* fix(listview): update index after pop * tests(listview): import future for type hints * ensure pop error is original index rather than normalized * fix(listview): update index after remove_items * update changelog * reinstate always_update to index reactive * Revert "reinstate always_update to index reactive" This reverts commit 68e205ee4f. * handle unchanged index without always_update * update changelog * update changelog * add docstrings --------- Co-authored-by: Will McGugan <willmcgugan@gmail.com> Co-authored-by: Darren Burns <darrenburns@users.noreply.github.com>
2025-10-17 02:38:12 +03:00 · 2024-11-28 16:00:52 +00:00
parent 79df474d14
commit 8c8057b5b5
3 changed files with 142 additions and 11 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,13 +5,19 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/)
 and this project adheres to [Semantic Versioning](http://semver.org/).

-### Unreleased
+## Unreleased

 ### Fixed

 - Fixed infinite loop in `Widget.anchor` https://github.com/Textualize/textual/pull/5290
 - Restores the ability to supply console markup to command list https://github.com/Textualize/textual/pull/5294
 - Fixed delayed App Resize event https://github.com/Textualize/textual/pull/5296
+- Fixed `ListView` not updating its index or highlighting after removing items https://github.com/Textualize/textual/issues/5114
+
+### Changed
+
+- `ListView.pop` now returns `AwaitComplete` rather than `AwaitRemove` https://github.com/Textualize/textual/pull/5135
+- `ListView.remove_items` now returns `AwaitComplete` rather than `AwaitRemove` https://github.com/Textualize/textual/pull/5135
 - Fixed ListView focus styling rule being too broad https://github.com/Textualize/textual/pull/5304
 - Fixed issue with auto-generated tab IDs https://github.com/Textualize/textual/pull/5298

@@ -46,6 +52,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

 - Fixed a glitch with the scrollbar that occurs when you hold `a` to add stopwatches in the tutorial app https://github.com/Textualize/textual/pull/5257

+
 ## [0.86.2] - 2024-11-18

 ### Fixed
--- a/src/textual/widgets/_list_view.py
+++ b/src/textual/widgets/_list_view.py
@@ -5,6 +5,7 @@ from typing import ClassVar, Iterable, Optional
 from typing_extensions import TypeGuard

 from textual._loop import loop_from_index
+from textual.await_complete import AwaitComplete
 from textual.await_remove import AwaitRemove
 from textual.binding import Binding, BindingType
 from textual.containers import VerticalScroll
@@ -280,7 +281,7 @@ class ListView(VerticalScroll, can_focus=True, can_focus_children=False):
        await_mount = self.mount(*items, before=index)
        return await_mount

-    def pop(self, index: Optional[int] = None) -> AwaitRemove:
+    def pop(self, index: Optional[int] = None) -> AwaitComplete:
        """Remove last ListItem from ListView or
           Remove ListItem from ListView by index

@@ -291,13 +292,31 @@ class ListView(VerticalScroll, can_focus=True, can_focus_children=False):
            An awaitable that yields control to the event loop until
                the DOM has been updated to reflect item being removed.
        """
-        if index is None:
-            await_remove = self.query("ListItem").last().remove()
-        else:
-            await_remove = self.query("ListItem")[index].remove()
-        return await_remove
+        if len(self) == 0:
+            raise IndexError("pop from empty list")

-    def remove_items(self, indices: Iterable[int]) -> AwaitRemove:
+        index = index if index is not None else -1
+        item_to_remove = self.query("ListItem")[index]
+        normalized_index = index if index >= 0 else index + len(self)
+
+        async def do_pop() -> None:
+            """Remove the item and update the highlighted index."""
+            await item_to_remove.remove()
+            if self.index is not None:
+                if normalized_index < self.index:
+                    self.index -= 1
+                elif normalized_index == self.index:
+                    old_index = self.index
+                    # Force a re-validation of the index
+                    self.index = self.index
+                    # If the index hasn't changed, the watcher won't be called
+                    # but we need to update the highlighted item
+                    if old_index == self.index:
+                        self.watch_index(old_index, self.index)
+
+        return AwaitComplete(do_pop())
+
+    def remove_items(self, indices: Iterable[int]) -> AwaitComplete:
        """Remove ListItems from ListView by indices

        Args:
@@ -308,8 +327,29 @@ class ListView(VerticalScroll, can_focus=True, can_focus_children=False):
        """
        items = self.query("ListItem")
        items_to_remove = [items[index] for index in indices]
-        await_remove = self.remove_children(items_to_remove)
-        return await_remove
+        normalized_indices = set(
+            index if index >= 0 else index + len(self) for index in indices
+        )
+
+        async def do_remove_items() -> None:
+            """Remove the items and update the highlighted index."""
+            await self.remove_children(items_to_remove)
+            if self.index is not None:
+                removed_before_highlighted = sum(
+                    1 for index in normalized_indices if index < self.index
+                )
+                if removed_before_highlighted:
+                    self.index -= removed_before_highlighted
+                elif self.index in normalized_indices:
+                    old_index = self.index
+                    # Force a re-validation of the index
+                    self.index = self.index
+                    # If the index hasn't changed, the watcher won't be called
+                    # but we need to update the highlighted item
+                    if old_index == self.index:
+                        self.watch_index(old_index, self.index)
+
+        return AwaitComplete(do_remove_items())

    def action_select_cursor(self) -> None:
        """Select the current item in the list."""
--- a/tests/listview/test_listview_remove_items.py
+++ b/tests/listview/test_listview_remove_items.py
@@ -1,8 +1,31 @@
+from __future__ import annotations
+
+import pytest
+
 from textual.app import App, ComposeResult
-from textual.widgets import ListView, ListItem, Label
+from textual.widgets import Label, ListItem, ListView
+
+
+class EmptyListViewApp(App[None]):
+    def compose(self) -> ComposeResult:
+        yield ListView()
+
+
+async def test_listview_pop_empty_raises_index_error():
+    app = EmptyListViewApp()
+    async with app.run_test() as pilot:
+        listview = pilot.app.query_one(ListView)
+        with pytest.raises(IndexError) as excinfo:
+            listview.pop()
+        assert "pop from empty list" in str(excinfo.value)


 class ListViewApp(App[None]):
+    def __init__(self, initial_index: int | None = None):
+        super().__init__()
+        self.initial_index = initial_index
+        self.highlighted = []
+
    def compose(self) -> ComposeResult:
        yield ListView(
            ListItem(Label("0")),
@@ -14,8 +37,15 @@ class ListViewApp(App[None]):
            ListItem(Label("6")),
            ListItem(Label("7")),
            ListItem(Label("8")),
+            initial_index=self.initial_index,
        )

+    def _on_list_view_highlighted(self, message: ListView.Highlighted) -> None:
+        if message.item is None:
+            self.highlighted.append(None)
+        else:
+            self.highlighted.append(str(message.item.children[0].renderable))
+

 async def test_listview_remove_items() -> None:
    """Regression test for https://github.com/Textualize/textual/issues/4735"""
@@ -27,6 +57,60 @@ async def test_listview_remove_items() -> None:
        assert len(listview) == 4


+@pytest.mark.parametrize(
+    "initial_index, pop_index, expected_new_index, expected_highlighted",
+    [
+        (2, 2, 2, ["2", "3"]),  # Remove highlighted item
+        (0, 0, 0, ["0", "1"]),  # Remove first item when highlighted
+        (8, None, 7, ["8", "7"]),  # Remove last item when highlighted
+        (4, 2, 3, ["4", "4"]),  # Remove item before the highlighted index
+        (4, -2, 4, ["4"]),  # Remove item after the highlighted index
+    ],
+)
+async def test_listview_pop_updates_index_and_highlighting(
+    initial_index, pop_index, expected_new_index, expected_highlighted
+) -> None:
+    """Regression test for https://github.com/Textualize/textual/issues/5114"""
+    app = ListViewApp(initial_index)
+    async with app.run_test() as pilot:
+        listview = pilot.app.query_one(ListView)
+
+        await listview.pop(pop_index)
+        await pilot.pause()
+
+        assert listview.index == expected_new_index
+        assert listview._nodes[expected_new_index].highlighted is True
+        assert app.highlighted == expected_highlighted
+
+
+@pytest.mark.parametrize(
+    "initial_index, remove_indices, expected_new_index, expected_highlighted",
+    [
+        (2, [2], 2, ["2", "3"]),  # Remove highlighted item
+        (0, [0], 0, ["0", "1"]),  # Remove first item when highlighted
+        (8, [-1], 7, ["8", "7"]),  # Remove last item when highlighted
+        (4, [2, 1], 2, ["4", "4"]),  # Remove items before the highlighted index
+        (4, [-2, 5], 4, ["4"]),  # Remove items after the highlighted index
+        (4, range(0, 9), None, ["4", None]),  # Remove all items
+    ],
+)
+async def test_listview_remove_items_updates_index_and_highlighting(
+    initial_index, remove_indices, expected_new_index, expected_highlighted
+) -> None:
+    """Regression test for https://github.com/Textualize/textual/issues/5114"""
+    app = ListViewApp(initial_index)
+    async with app.run_test() as pilot:
+        listview = pilot.app.query_one(ListView)
+
+        await listview.remove_items(remove_indices)
+        await pilot.pause()
+
+        assert listview.index == expected_new_index
+        if expected_new_index is not None:
+            assert listview._nodes[expected_new_index].highlighted is True
+        assert app.highlighted == expected_highlighted
+
+
 if __name__ == "__main__":
    app = ListViewApp()
    app.run()