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 68e205e.

* handle unchanged index without always_update

* update changelog

* update changelog

* add docstrings

---------

Co-authored-by: Will McGugan <[email protected]>
Co-authored-by: Darren Burns <[email protected]>

Author: 3 people

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

src/textual/widgets/_list_view.py

@@ -5,6 +5,7 @@
 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 @@ def insert(self, index: int, items: Iterable[ListItem]) -> AwaitMount:
         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 @@ def pop(self, index: Optional[int] = None) -> AwaitRemove:
             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
-
-    def remove_items(self, indices: Iterable[int]) -> AwaitRemove:
+        if len(self) == 0:
+            raise IndexError("pop from empty list")
+
+        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 @@ def remove_items(self, indices: Iterable[int]) -> AwaitRemove:
         """
         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."""

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 @@ def compose(self) -> ComposeResult:
             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()