diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c2d544bd3..6de8ba68fe 100644 --- 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 diff --git a/src/textual/widgets/_list_view.py b/src/textual/widgets/_list_view.py index a5879b7c44..b8c36bdd4c 100644 --- a/src/textual/widgets/_list_view.py +++ b/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.""" diff --git a/tests/listview/test_listview_remove_items.py b/tests/listview/test_listview_remove_items.py index 43501cdf19..48a03633dd 100644 --- 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 @@ 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()