From 8db8d69b806fd0548b7028b7564e37432c8b07a4 Mon Sep 17 00:00:00 2001 From: Ben Davidson Date: Tue, 6 Oct 2026 22:31:21 +1030 Subject: [PATCH] Halt a running glide before a landing that finds the pane already in place A match landing scrolls with `scroll_to_region`, which measures from the pane's live position and issues nothing when the delta is zero. Textual only stops a running scroll animation inside `_scroll_to`, so a landing that found the pane already where it wanted it left an earlier glide running, and the preview ended on the earlier target. Down then Up in the results list, the second press landing before the first glide had moved the pane, left the preview on the table with the cursor on the section above it. `MatchAwareScroll.halt_glide` drops a running `scroll_y` animation where it is (animating to the current value, which avoids `force_stop_animation`'s jump to the old target), and the match landing and the n/b stop scroll call it first. Under the test harness the race reproduces at a 50 ms gap (2 of 4 on main, 0 of 28 with this change); a live terminal did not hit it in 83 tries, since there the glide has usually moved before the second landing. --- fnd/tui/match_navigator.py | 4 +++ fnd/tui/preview_scroll.py | 2 ++ fnd/tui/preview_scrollbar.py | 9 ++++++ tests/test_match_nav_integration.py | 50 ++++++++++++++++++++++++++++- 4 files changed, 64 insertions(+), 1 deletion(-) diff --git a/fnd/tui/match_navigator.py b/fnd/tui/match_navigator.py index fba9cfa7..7f7db41a 100644 --- a/fnd/tui/match_navigator.py +++ b/fnd/tui/match_navigator.py @@ -883,6 +883,8 @@ def _go(self, *, forward: bool) -> None: def _scroll_to_stop(self, pane: VerticalScroll, top_y: int, vh: int) -> None: from textual.geometry import Region + from fnd.tui.preview_scrollbar import MatchAwareScroll + # The anchor IS the view's top: views tile by exactly one viewport, so a # hop covers a screenful and the border's count of screenfuls and of # presses are the same number. Offsetting the match down the viewport @@ -899,6 +901,8 @@ def _scroll_to_stop(self, pane: VerticalScroll, top_y: int, vh: int) -> None: if preview is not None: preview.begin_reconcile_scroll() try: + if isinstance(pane, MatchAwareScroll): + pane.halt_glide() pane.scroll_to_region(region, top=True, animate=False, immediate=True) self._app._diag_log(f"scroll site=nb_stop top_y={top_y}") finally: diff --git a/fnd/tui/preview_scroll.py b/fnd/tui/preview_scroll.py index 79253fc8..39a62405 100644 --- a/fnd/tui/preview_scroll.py +++ b/fnd/tui/preview_scroll.py @@ -850,6 +850,8 @@ def _scroll_pane_to_match_region( region = Region( region.x, max(0, region.y - margin), region.width, region.height + margin ) + if isinstance(pane, MatchAwareScroll): + pane.halt_glide() start = int(pane.scroll_offset.y) delta = pane.scroll_to_region(region, top=True, animate=animate, immediate=not animate) self._host.diag_log(f"scroll site=match region_y={region.y} animate={animate}") diff --git a/fnd/tui/preview_scrollbar.py b/fnd/tui/preview_scrollbar.py index 3666c762..a8169bca 100644 --- a/fnd/tui/preview_scrollbar.py +++ b/fnd/tui/preview_scrollbar.py @@ -415,6 +415,15 @@ def action_scroll_up(self) -> None: return super().action_scroll_up() + def halt_glide(self) -> None: + """Stop a scroll animation where it is. A scroll that finds the pane in + place issues nothing, so a glide to an earlier target would carry on; + animating to the current value drops it without ``force_stop``'s jump.""" + if not self.app.animator.is_being_animated(self, "scroll_y"): + return + self.animate("scroll_y", self.scroll_y, duration=0) + self.scroll_target_y = self.scroll_y + #: ``(widget, virtual_y)`` for a chunk just below content being revealed #: ABOVE the viewport. Every layout that pushes that widget further down is #: the prepend landing, and the pane scrolls by exactly that so the document diff --git a/tests/test_match_nav_integration.py b/tests/test_match_nav_integration.py index 1d00a38e..d4a93228 100644 --- a/tests/test_match_nav_integration.py +++ b/tests/test_match_nav_integration.py @@ -24,7 +24,7 @@ from fnd.config import Config, load from fnd.index import build_index from fnd.tui import FNDApp -from tests._pilot_wait import safe_press, settle, wait_until +from tests._pilot_wait import safe_press, settle, wait_stable, wait_until def _write(p: Path, body: str) -> None: @@ -550,6 +550,54 @@ async def test_a_hand_over_onto_the_row_the_cursor_already_holds_still_lands( ) +@pytest.mark.asyncio +@pytest.mark.parametrize("site", ["landing", "nb_stop"]) +async def test_a_scroll_to_where_the_pane_already_is_still_ends_a_glide( + cfg: Config, flashcards_index: Path, site: str +) -> None: + """A scroll that finds the pane already in place still supersedes a glide elsewhere.""" + from textual.geometry import Region + + app = FNDApp(index_dir=flashcards_index, config=cfg, collection="notes", initial_query="CRC") + async with app.run_test(size=(110, 24)) as pilot: + pane = app.query_one("#preview_pane", VerticalScroll) + await wait_until( + pilot, + lambda: ( + _focus_seq(app) is not None + and not app._preview_scroll.is_settling + and not app.animator.is_being_animated(pane, "scroll_y") + ), + timeout=30.0, + message="the preview never landed", + ) + await wait_stable( + pilot, + lambda: (pane.scroll_y, app._preview_scroll.is_settling), + rounds=6, + message="the landing never held still", + ) + here = int(pane.scroll_y) + assert here > 0, "the landing must leave room to glide towards the top" + pane.scroll_to(y=0, animate=True, duration=0.3, immediate=True) + assert app.animator.is_being_animated(pane, "scroll_y") + + if site == "landing": + app._preview_scroll_structural._scroll_pane_to_match_region( + pane, Region(0, here, 1, 1), 0, animate=True + ) + else: + app._match_nav._scroll_to_stop(pane, here, pane.scrollable_content_region.height) + + await wait_until( + pilot, + lambda: not app.animator.is_being_animated(pane, "scroll_y"), + timeout=5.0, + message="the glide never ended", + ) + assert int(pane.scroll_y) == here + + @pytest.fixture def three_section_index(tmp_path: Path, tmp_index_dir: Path) -> Path: """A long middle section, so the last section's final stop sits on the