diff --git a/CHANGELOG.md b/CHANGELOG.md index ef410b0749..f3ae4b5d98 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Added `Containers.HorizontalGroup` and `Containers.VerticalGroup` https://github.com/Textualize/textual/pull/5113 - Added `$`, `£`, `€`, `(`, `)` symbols to Digits https://github.com/Textualize/textual/pull/5113 - Added `Button.action` parameter to invoke action when clicked https://github.com/Textualize/textual/pull/5113 +- Added `immediate` parameter to scroll methods https://github.com/Textualize/textual/pull/5164 +- Added `textual._loop.loop_from_index` https://github.com/Textualize/textual/pull/5164 + +### Fixed + +- Fixed glitchy ListView https://github.com/Textualize/textual/issues/5163 ## [0.84.0] - 2024-10-22 diff --git a/src/textual/_loop.py b/src/textual/_loop.py index 7a057fc214..67546999c0 100644 --- a/src/textual/_loop.py +++ b/src/textual/_loop.py @@ -1,6 +1,6 @@ from __future__ import annotations -from typing import Iterable, TypeVar +from typing import Iterable, Literal, Sequence, TypeVar T = TypeVar("T") @@ -43,3 +43,44 @@ def loop_first_last(values: Iterable[T]) -> Iterable[tuple[bool, bool, T]]: first = False previous_value = value yield first, True, previous_value + + +def loop_from_index( + values: Sequence[T], + index: int, + direction: Literal[-1, +1] = +1, + wrap: bool = True, +) -> Iterable[tuple[int, T]]: + """Iterate over values in a sequence from a given starting index, potentially wrapping the index + if it would go out of bounds. + + Note that the first value to be yielded is a step from `index`, and `index` will be yielded *last*. + + + Args: + values: A sequence of values. + index: Starting index. + direction: Direction to move index (+1 for forward, -1 for backward). + bool: Should the index wrap when out of bounds? + + Yields: + A tuple of index and value from the sequence. + """ + # Sanity check for devs who miss the typing errors + assert direction in (-1, +1), "direction must be -1 or +1" + count = len(values) + if wrap: + for _ in range(count): + index = (index + direction) % count + yield (index, values[index]) + else: + if direction == +1: + for _ in range(count): + if (index := index + 1) >= count: + break + yield (index, values[index]) + else: + for _ in range(count): + if (index := index - 1) < 0: + break + yield (index, values[index]) diff --git a/src/textual/_profile.py b/src/textual/_profile.py index 279a873ec0..3e880cf15c 100644 --- a/src/textual/_profile.py +++ b/src/textual/_profile.py @@ -16,4 +16,4 @@ def timer(subject: str = "time") -> Generator[None, None, None]: yield elapsed = perf_counter() - start elapsed_ms = elapsed * 1000 - log(f"{subject} elapsed {elapsed_ms:.2f}ms") + log(f"{subject} elapsed {elapsed_ms:.4f}ms") diff --git a/src/textual/_widget_navigation.py b/src/textual/_widget_navigation.py index c547d01be0..302a332090 100644 --- a/src/textual/_widget_navigation.py +++ b/src/textual/_widget_navigation.py @@ -8,12 +8,13 @@ from __future__ import annotations -from functools import partial from itertools import count from typing import Literal, Protocol, Sequence from typing_extensions import TypeAlias +from textual._loop import loop_from_index + class Disableable(Protocol): """Non-widgets that have an enabled/disabled status.""" @@ -105,7 +106,6 @@ def find_next_enabled( candidates: Sequence[Disableable], anchor: int | None, direction: Direction, - with_anchor: bool = False, ) -> int | None: """Find the next enabled object if we're currently at the given anchor. @@ -118,8 +118,6 @@ def find_next_enabled( enabled object. direction: The direction in which to traverse the candidates when looking for the next enabled candidate. - with_anchor: Consider the anchor position as the first valid position instead of - the last one. Returns: The next enabled object. If none are available, return the anchor. @@ -134,17 +132,10 @@ def find_next_enabled( ) return None - start = anchor + direction if not with_anchor else anchor - key_function = partial( - get_directed_distance, - start=start, - direction=direction, - wrap_at=len(candidates), - ) - enabled_candidates = [ - index for index, candidate in enumerate(candidates) if not candidate.disabled - ] - return min(enabled_candidates, key=key_function, default=anchor) + for index, candidate in loop_from_index(candidates, anchor, direction, wrap=True): + if not candidate.disabled: + return index + return anchor def find_next_enabled_no_wrap( diff --git a/src/textual/scroll_view.py b/src/textual/scroll_view.py index 56b947172f..42c89ebb23 100644 --- a/src/textual/scroll_view.py +++ b/src/textual/scroll_view.py @@ -123,6 +123,7 @@ def scroll_to( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll to a given (absolute) coordinate, optionally animating. @@ -136,6 +137,8 @@ def scroll_to( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ self._scroll_to( diff --git a/src/textual/widget.py b/src/textual/widget.py index ccc9e14f25..d2350db478 100644 --- a/src/textual/widget.py +++ b/src/textual/widget.py @@ -2381,6 +2381,7 @@ def scroll_to( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll to a given (absolute) coordinate, optionally animating. @@ -2394,22 +2395,37 @@ def scroll_to( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. Note: The call to scroll is made after the next refresh. """ - self.call_after_refresh( - self._scroll_to, - x, - y, - animate=animate, - speed=speed, - duration=duration, - easing=easing, - force=force, - on_complete=on_complete, - level=level, - ) + if immediate: + self._scroll_to( + x, + y, + animate=animate, + speed=speed, + duration=duration, + easing=easing, + force=force, + on_complete=on_complete, + level=level, + ) + else: + self.call_after_refresh( + self._scroll_to, + x, + y, + animate=animate, + speed=speed, + duration=duration, + easing=easing, + force=force, + on_complete=on_complete, + level=level, + ) def scroll_relative( self, @@ -2423,6 +2439,7 @@ def scroll_relative( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll relative to current position. @@ -2436,6 +2453,8 @@ def scroll_relative( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ self.scroll_to( None if x is None else (self.scroll_x + x), @@ -2447,6 +2466,7 @@ def scroll_relative( force=force, on_complete=on_complete, level=level, + immediate=immediate, ) def scroll_home( @@ -2459,6 +2479,7 @@ def scroll_home( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll to home position. @@ -2470,6 +2491,8 @@ def scroll_home( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ if speed is None and duration is None: duration = 1.0 @@ -2483,6 +2506,7 @@ def scroll_home( force=force, on_complete=on_complete, level=level, + immediate=immediate, ) def scroll_end( @@ -2495,6 +2519,7 @@ def scroll_end( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll to the end of the container. @@ -2506,6 +2531,8 @@ def scroll_end( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ if speed is None and duration is None: duration = 1.0 @@ -2542,6 +2569,7 @@ def scroll_left( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll one cell left. @@ -2553,6 +2581,8 @@ def scroll_left( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ self.scroll_to( x=self.scroll_target_x - 1, @@ -2563,6 +2593,7 @@ def scroll_left( force=force, on_complete=on_complete, level=level, + immediate=immediate, ) def _scroll_left_for_pointer( @@ -2615,6 +2646,7 @@ def scroll_right( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll one cell right. @@ -2626,6 +2658,8 @@ def scroll_right( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ self.scroll_to( x=self.scroll_target_x + 1, @@ -2636,6 +2670,7 @@ def scroll_right( force=force, on_complete=on_complete, level=level, + immediate=immediate, ) def _scroll_right_for_pointer( @@ -2688,6 +2723,7 @@ def scroll_down( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll one line down. @@ -2699,6 +2735,8 @@ def scroll_down( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ self.scroll_to( y=self.scroll_target_y + 1, @@ -2709,6 +2747,7 @@ def scroll_down( force=force, on_complete=on_complete, level=level, + immediate=immediate, ) def _scroll_down_for_pointer( @@ -2761,6 +2800,7 @@ def scroll_up( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll one line up. @@ -2772,6 +2812,8 @@ def scroll_up( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ self.scroll_to( y=self.scroll_target_y - 1, @@ -2974,6 +3016,7 @@ def scroll_to_widget( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> bool: """Scroll scrolling to bring a widget in to view. @@ -2988,6 +3031,8 @@ def scroll_to_widget( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. Returns: `True` if any scrolling has occurred in any descendant, otherwise `False`. @@ -3014,6 +3059,7 @@ def scroll_to_widget( force=force, on_complete=on_complete, level=level, + immediate=immediate, ) if scroll_offset: scrolled = True @@ -3053,6 +3099,7 @@ def scroll_to_region( level: AnimationLevel = "basic", x_axis: bool = True, y_axis: bool = True, + immediate: bool = False, ) -> Offset: """Scrolls a given region in to view, if required. @@ -3073,6 +3120,8 @@ def scroll_to_region( level: Minimum level required for the animation to take place (inclusive). x_axis: Allow scrolling on X axis? y_axis: Allow scrolling on Y axis? + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. Returns: The distance that was scrolled. @@ -3133,6 +3182,7 @@ def clamp_delta(delta: Offset) -> Offset: force=force, on_complete=on_complete, level=level, + immediate=immediate, ) return delta @@ -3147,6 +3197,7 @@ def scroll_visible( force: bool = False, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll the container to make this widget visible. @@ -3159,11 +3210,12 @@ def scroll_visible( force: Force scrolling even when prohibited by overflow styling. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ parent = self.parent if isinstance(parent, Widget): - self.call_after_refresh( - self.screen.scroll_to_widget, + self.screen.scroll_to_widget( self, animate=animate, speed=speed, @@ -3173,6 +3225,7 @@ def scroll_visible( force=force, on_complete=on_complete, level=level, + immediate=immediate, ) def scroll_to_center( @@ -3187,6 +3240,7 @@ def scroll_to_center( origin_visible: bool = True, on_complete: CallbackType | None = None, level: AnimationLevel = "basic", + immediate: bool = False, ) -> None: """Scroll this widget to the center of self. @@ -3202,9 +3256,11 @@ def scroll_to_center( origin_visible: Ensure that the top left corner of the widget remains visible after the scroll. on_complete: A callable to invoke when the animation is finished. level: Minimum level required for the animation to take place (inclusive). + immediate: If `False` the scroll will be deferred until after a screen refresh, + set to `True` to scroll immediately. """ - self.call_after_refresh( - self.scroll_to_widget, + + self.scroll_to_widget( widget=widget, animate=animate, speed=speed, @@ -3215,6 +3271,7 @@ def scroll_to_center( origin_visible=origin_visible, on_complete=on_complete, level=level, + immediate=immediate, ) def can_view(self, widget: Widget) -> bool: diff --git a/src/textual/widgets/_list_view.py b/src/textual/widgets/_list_view.py index a60ddf8468..b92f1bf773 100644 --- a/src/textual/widgets/_list_view.py +++ b/src/textual/widgets/_list_view.py @@ -4,7 +4,7 @@ from typing_extensions import TypeGuard -from textual import _widget_navigation +from textual._loop import loop_from_index from textual.await_remove import AwaitRemove from textual.binding import Binding, BindingType from textual.containers import VerticalScroll @@ -38,7 +38,7 @@ class ListView(VerticalScroll, can_focus=True, can_focus_children=False): | down | Move the cursor down. | """ - index = reactive[Optional[int]](0, always_update=True, init=False) + index = reactive[Optional[int]](None, init=False) """The index of the currently highlighted item.""" class Highlighted(Message): @@ -117,17 +117,20 @@ def __init__( super().__init__( *children, name=name, id=id, classes=classes, disabled=disabled ) - # Set the index to the given initial index, or the first available index after. - self._index = _widget_navigation.find_next_enabled( - children, - anchor=initial_index if initial_index is not None else None, - direction=1, - with_anchor=True, - ) + self._initial_index = initial_index def _on_mount(self, _: Mount) -> None: """Ensure the ListView is fully-settled after mounting.""" - self.index = self._index + + if self._initial_index is not None and self.children: + index = self._initial_index + if index >= len(self.children): + index = 0 + if self._nodes[index].disabled: + for index, node in loop_from_index(self._nodes, index, wrap=True): + if not node.disabled: + break + self.index = index @property def highlighted_child(self) -> ListItem | None: @@ -165,16 +168,30 @@ def _is_valid_index(self, index: int | None) -> TypeGuard[int]: def watch_index(self, old_index: int | None, new_index: int | None) -> None: """Updates the highlighting when the index changes.""" + + if new_index is not None: + selected_widget = self._nodes[new_index] + if selected_widget.region: + self.scroll_to_widget(self._nodes[new_index], animate=False) + else: + # Call after refresh to permit a refresh operation + self.call_after_refresh( + self.scroll_to_widget, selected_widget, animate=False + ) + if self._is_valid_index(old_index): old_child = self._nodes[old_index] assert isinstance(old_child, ListItem) old_child.highlighted = False - if self._is_valid_index(new_index) and not self._nodes[new_index].disabled: + if ( + new_index is not None + and self._is_valid_index(new_index) + and not self._nodes[new_index].disabled + ): new_child = self._nodes[new_index] assert isinstance(new_child, ListItem) new_child.highlighted = True - self._scroll_highlighted_region() self.post_message(self.Highlighted(self, new_child)) else: self.post_message(self.Highlighted(self, None)) @@ -190,8 +207,6 @@ def extend(self, items: Iterable[ListItem]) -> AwaitMount: until the DOM has been updated with the new child items. """ await_mount = self.mount(*items) - if len(self) == 1: - self.index = 0 return await_mount def append(self, item: ListItem) -> AwaitMount: @@ -271,27 +286,28 @@ def action_select_cursor(self) -> None: def action_cursor_down(self) -> None: """Highlight the next item in the list.""" - candidate = _widget_navigation.find_next_enabled( - self._nodes, - anchor=self.index, - direction=1, - ) - if self.index is not None and candidate is not None and candidate < self.index: - return # Avoid wrapping around. - - self.index = candidate + if self.index is None: + if self._nodes: + self.index = 0 + else: + index = self.index + for index, item in loop_from_index(self._nodes, self.index, wrap=False): + if not item.disabled: + self.index = index + break def action_cursor_up(self) -> None: """Highlight the previous item in the list.""" - candidate = _widget_navigation.find_next_enabled( - self._nodes, - anchor=self.index, - direction=-1, - ) - if self.index is not None and candidate is not None and candidate > self.index: - return # Avoid wrapping around. - - self.index = candidate + if self.index is None: + if self._nodes: + self.index = len(self._nodes) - 1 + else: + for index, item in loop_from_index( + self._nodes, self.index, direction=-1, wrap=False + ): + if not item.disabled: + self.index = index + break def _on_list_item__child_clicked(self, event: ListItem._ChildClicked) -> None: event.stop() @@ -299,13 +315,6 @@ def _on_list_item__child_clicked(self, event: ListItem._ChildClicked) -> None: self.index = self._nodes.index(event.item) self.post_message(self.Selected(self, event.item)) - def _scroll_highlighted_region(self) -> None: - """Used to keep the highlighted index within vision""" - if self.highlighted_child is not None: - self.call_after_refresh( - self.scroll_to_widget, self.highlighted_child, animate=False - ) - def __len__(self) -> int: """Compute the length (in number of items) of the list view.""" return len(self._nodes) diff --git a/tests/listview/test_listview_initial_index.py b/tests/listview/test_listview_initial_index.py index 515de4f044..ab2eed573e 100644 --- a/tests/listview/test_listview_initial_index.py +++ b/tests/listview/test_listview_initial_index.py @@ -39,4 +39,4 @@ def compose(self) -> ComposeResult: app = ListViewDisabledItemsApp() async with app.run_test() as pilot: list_view = pilot.app.query_one(ListView) - assert list_view._index == expected_index + assert list_view.index == expected_index diff --git a/tests/snapshot_tests/snapshot_apps/listview_index.py b/tests/snapshot_tests/snapshot_apps/listview_index.py index 1aa88aec3b..9027e2a3e0 100644 --- a/tests/snapshot_tests/snapshot_apps/listview_index.py +++ b/tests/snapshot_tests/snapshot_apps/listview_index.py @@ -22,7 +22,9 @@ def compose(self) -> ComposeResult: async def watch_data(self, data: "list[int]") -> None: await self._menu.remove_children() await self._menu.extend((ListItem(Label(str(value))) for value in data)) - self._menu.index = len(self._menu) - 1 + + new_index = len(self._menu) - 1 + self._menu.index = new_index async def on_ready(self): self.data = list(range(0, 30, 2)) diff --git a/tests/test_loop.py b/tests/test_loop.py index 87ef97da76..65b0aa0097 100644 --- a/tests/test_loop.py +++ b/tests/test_loop.py @@ -1,4 +1,4 @@ -from textual._loop import loop_first, loop_first_last, loop_last +from textual._loop import loop_first, loop_first_last, loop_from_index, loop_last def test_loop_first(): @@ -26,3 +26,40 @@ def test_loop_first_last(): assert next(iterable) == (False, False, "oranges") assert next(iterable) == (False, False, "pears") assert next(iterable) == (False, True, "lemons") + + +def test_loop_from_index(): + assert list(loop_from_index("abcdefghij", 3)) == [ + (4, "e"), + (5, "f"), + (6, "g"), + (7, "h"), + (8, "i"), + (9, "j"), + (0, "a"), + (1, "b"), + (2, "c"), + (3, "d"), + ] + + assert list(loop_from_index("abcdefghij", 3, direction=-1)) == [ + (2, "c"), + (1, "b"), + (0, "a"), + (9, "j"), + (8, "i"), + (7, "h"), + (6, "g"), + (5, "f"), + (4, "e"), + (3, "d"), + ] + + assert list(loop_from_index("abcdefghij", 3, wrap=False)) == [ + (4, "e"), + (5, "f"), + (6, "g"), + (7, "h"), + (8, "i"), + (9, "j"), + ] diff --git a/tests/test_widget_navigation.py b/tests/test_widget_navigation.py index a322f3846f..44f8b17159 100644 --- a/tests/test_widget_navigation.py +++ b/tests/test_widget_navigation.py @@ -142,16 +142,10 @@ def test_find_next_enabled_no_wrap(candidates, anchor, direction, result): @pytest.mark.parametrize( ["function", "start", "direction"], [ - (find_next_enabled, 0, 1), - (find_next_enabled, 0, -1), (find_next_enabled_no_wrap, 0, 1), (find_next_enabled_no_wrap, 0, -1), - (find_next_enabled, 1, 1), - (find_next_enabled, 1, -1), (find_next_enabled_no_wrap, 1, 1), (find_next_enabled_no_wrap, 1, -1), - (find_next_enabled, 2, 1), - (find_next_enabled, 2, -1), (find_next_enabled_no_wrap, 2, 1), (find_next_enabled_no_wrap, 2, -1), ],