diff --git a/meshterm/ui/contacts_screen.py b/meshterm/ui/contacts_screen.py index 5e5a20d..f899863 100644 --- a/meshterm/ui/contacts_screen.py +++ b/meshterm/ui/contacts_screen.py @@ -175,12 +175,13 @@ async def open_contacts( ) -> None: """Open the interactive contacts list; Enter opens Node detail, Esc leaves. - Runs the show/open/reshow loop: the sortable list, then whichever node's detail page - Enter commits (or the purge flow the tail action opens), then the list again (same sort, - the highlight kept on its row) — until Esc, or the ``Back`` row, backs out of the list - itself. Anything that removes a contact — a purge here, or the detail page's own - single-contact ``Remove contact…`` — re-reads the device and rebuilds the list, so the - contacts that went are gone from it. + The list stays pushed for the whole visit, so whichever node's detail page Enter commits + (or the purge flow the tail action opens) nests *above* it and Esc from there is one pop + back onto the row it was opened from — same sort, same filter, same scroll. Anything that + removes a contact — a purge here, or the detail page's own single-contact + ``Remove contact…`` — ends the visit, re-reads the device and starts a fresh one, so the + contacts that went are gone from the list. That rebuild is the deliberate exception to + keeping the screen: the rows it was holding a place in no longer exist. Args: ctx: Shared application context (must be in the interactive menu). @@ -201,35 +202,35 @@ async def open_contacts( if not isinstance(ctx.ui, TuiUi): # pragma: no cover - guarded by the menu-only caller raise RuntimeError("the interactive contacts list is only available in the menu") session = ctx.ui.session - # One screen for the whole visit: re-running it keeps the highlight (and any sort or - # filter) on the row the user just opened a detail for, rather than snapping to the top. screen = ContactsScreen(self_name, self_key, contacts, prefix_bytes, counts, sort) while True: - chosen = await session.run_screen(screen) - if chosen is CANCEL or chosen is None: # Esc + rebuild = False + async with session.stay(screen) as visit: + while True: + chosen = await visit.result() + if chosen is CANCEL or chosen is None: # Esc + return + if chosen == _PURGE: + # The purge picker and its confirm float over the list, which is already + # the backdrop. A purge that removed anything invalidates the contacts + # cache, so the list is re-read and rebuilt — the one deliberate reset in + # this screen's life, because the rows it was keeping a place in are gone. + if await _purge_stale(ctx, self_key): + rebuild = True + break + continue + # The own-node sentinel opens our own node's page; any other value is a + # Contact. The page nests above this list and can also *delete* the contact + # it details (its ``Remove contact…`` row); when it does, it says so on the + # way out and the list rebuilds without the row — the same re-read the purge + # does, one contact at a time. + if await open_node_detail(ctx, None if chosen == YOU else chosen): + rebuild = True + break + if not rebuild: return - if chosen == _PURGE: - # The purge picker and its confirm float over the list, so re-push it as the - # backdrop while they run (run_screen just popped it). A purge that removed - # anything invalidates the contacts cache, so re-read and rebuild the list. - session.push(screen) - try: - removed = await _purge_stale(ctx, self_key) - finally: - session.pop(screen) - if removed: - contacts = await ctx.devstate.contacts() - screen = ContactsScreen( - self_name, self_key, contacts, prefix_bytes, counts, sort - ) - continue - # The own-node sentinel opens our own node's page; any other value is a Contact. - # The page can also *delete* the contact it details (its ``Remove contact…`` row); - # when it does, it says so on the way out and the list rebuilds without the row — - # the same re-read the purge does, one contact at a time. - if await open_node_detail(ctx, None if chosen == YOU else chosen): - contacts = await ctx.devstate.contacts() - screen = ContactsScreen(self_name, self_key, contacts, prefix_bytes, counts, sort) + contacts = await ctx.devstate.contacts() + screen = ContactsScreen(self_name, self_key, contacts, prefix_bytes, counts, sort) async def _purge_stale(ctx: "AppContext", self_key: str) -> int: diff --git a/meshterm/ui/node_detail_screen.py b/meshterm/ui/node_detail_screen.py index dd69c5a..399b547 100644 --- a/meshterm/ui/node_detail_screen.py +++ b/meshterm/ui/node_detail_screen.py @@ -1362,57 +1362,67 @@ async def open_node_detail(ctx: "AppContext", contact: Optional["Contact"]) -> b title = f"Node — {label}" if not you else f"Node — {label} (you)" # Whether the visit ended by deleting the contact — the caller's cue to rebuild its list. contact_removed = False - while True: - screen = NodeDetailScreen( - title=title, - header=header, - info_rows=info_rows, - tabs=tabs, - minimap=minimap, - map_caption=map_caption, - routes=routes_view, - info_actions=info_actions, - trace_action=trace_action, - ) - action = await session.run_screen(screen) - if action is CANCEL or action is None: - break - if action == "trace": - await open_trace(ctx, name or key, initial_spec=screen.selected_spec()) - elif action == "map": - if markers: - # Open centred on this node — the very spot the inline preview showed — - # not wherever the global map was last left — with the find filter seeded - # to it, so this node lights among the rest. Seed only when its label - # actually matches a marker; a needle nothing matches would dim everything. - # (The map action only exists when the node has a fix, so lat/lon are set.) - needle = label.casefold() - await open_map( - ctx, - markers, - focus=(lat, lon), - find=label if any(needle in m.label.casefold() for m in markers) else None, - ) - elif action == "share": - await show_contact_card(ctx, label, full_key, adv_type) - elif action == "timemachine": - if you: - await open_timemachine_self(ctx) - else: - await open_timemachine_node(ctx, node_id, label) - elif action == "remove": - # The confirm floats over this page (run_screen just popped it), so re-push it - # as the backdrop for the dialog's lifetime — the reader confirms against the - # node they are looking at. A removal ends the visit: there is no contact left - # to detail, and the list we return to rebuilds without it. - assert contact is not None # the row only exists for a real contact - session.push(screen) - try: - contact_removed = await _remove_contact(ctx, contact, self_key, label) - finally: - session.pop(screen) - if contact_removed: + # One screen for the whole visit. The page is a *hub* — every action on it opens + # something else and comes back — so it stays pushed while each of them runs + # (``session.stay``) instead of being rebuilt per round. Rebuilding cost the open tab + # and the cursor every time: Tab to Routes, pick a route, trace it, come back, and you + # were on Info at the top again, two keypresses from the route you were working + # through. Keeping the object keeps all of it. It also means the peers below *nest* + # above this page rather than replacing it, so Esc from a trace lands back here; ^W is + # what leaves the whole excursion at once. + screen = NodeDetailScreen( + title=title, + header=header, + info_rows=info_rows, + tabs=tabs, + minimap=minimap, + map_caption=map_caption, + routes=routes_view, + info_actions=info_actions, + trace_action=trace_action, + ) + async with session.stay(screen) as visit: + while True: + action = await visit.result() + if action is CANCEL or action is None: break + if action == "trace": + await open_trace(ctx, name or key, initial_spec=screen.selected_spec()) + elif action == "map": + if markers: + # Open centred on this node — the very spot the inline preview showed — + # not wherever the global map was last left — with the find filter seeded + # to it, so this node lights among the rest. Seed only when its label + # actually matches a marker; a needle nothing matches would dim + # everything. (The map action only exists when the node has a fix, so + # lat/lon are set.) + needle = label.casefold() + await open_map( + ctx, + markers, + focus=(lat, lon), + find=( + label + if any(needle in m.label.casefold() for m in markers) + else None + ), + ) + elif action == "share": + await show_contact_card(ctx, label, full_key, adv_type) + elif action == "timemachine": + if you: + await open_timemachine_self(ctx) + else: + await open_timemachine_node(ctx, node_id, label) + elif action == "remove": + # The confirm floats over this page, which is already the backdrop — the + # reader confirms against the node they are looking at. A removal ends the + # visit: there is no contact left to detail, and the list we return to + # rebuilds without it. + assert contact is not None # the row only exists for a real contact + contact_removed = await _remove_contact(ctx, contact, self_key, label) + if contact_removed: + break if minimap is not None: # The preview's braille may have smeared the terminal (double-width fallback # glyphs prompt_toolkit's diff can't see); force one clean repaint of the list diff --git a/meshterm/ui/records_screen.py b/meshterm/ui/records_screen.py index 23ae130..153f4f1 100644 --- a/meshterm/ui/records_screen.py +++ b/meshterm/ui/records_screen.py @@ -848,58 +848,57 @@ async def open_records(ctx: "AppContext") -> dict: wrap=False, hscroll=True, # a long walk slides under ←→ instead of dying at the fold ) - picked = await session.run_screen(browser) - if picked is CANCEL or picked is None: # Esc - return {"records": total} - # Every pick opens a dialog that belongs *over* the trophy case — the discipline - # picker, the delete confirms, a record's floating story. run_screen just popped the - # browser, so re-push it as the static backdrop: it stays drawn full-frame behind the - # dialog (which, run over the bare stack, would otherwise draw over a blank frame — or, - # alone and floating, become the frame). This is the device picker's trick - # (confirm_startup) and what RecordDialog's "floating over the screen beneath" assumes. - session.push(browser) - trace_spec: Optional[str] = None - try: - verb = picked[0] - if verb == "del_cat": - await delete_category_flow() - elif verb == "del_all": - if total and await session.typed_confirm( - f"This deletes all {total} records — every discipline, every width. " - "They can only be re-earned by walking them again.", - "delete", - title="Delete all records", - ): - ctx.repo.delete_discoveries() - else: - _verb, category, rank, record = picked - far_label, far_id, shape = walk_drawing(record) - action = await session.run_screen(RecordDialog( - record, category, rank, - resolve=resolve, device_label=device_label, device_hash=device_hash, - far_label=far_label, far_id=far_id, shape=shape, - reliability=walk_reliability(far_id, far_label), - type_of=lambda node_id: node_geo(node_id)[1], - )) - if action == "trace": - trace_spec = record.spec - elif action == "delete": - sure = await ctx.ui.dialog( - f"Delete this {category.title} record?", - [("Cancel", False), ("Delete", True)], - title="Delete record", - default=1, - destructive=True, - ) - if sure: - ctx.repo.delete_discovery(record.id) - finally: - session.pop(browser) - # Tracing a path is a full hand-off to the Trace screen, not a dialog over the - # trophy case, so it runs only once the browser backdrop is down — and the - # browser does not reopen behind it: when the trace screen closes, this whole - # flow returns, landing the user on the main menu instead of a trophy-case → - # trace → trophy-case stack that takes many Escs to climb out of. - if trace_spec is not None: - await open_trace_path(ctx, spec=trace_spec) - return {"records": len(ctx.repo.discoveries())} + # The browser stays pushed for the whole visit, so every dialog that belongs *over* + # the trophy case — the discipline picker, the delete confirms, a record's floating + # story — already has it drawn full-frame behind them, and the cursor is still on the + # record just read when they close. The loop only leaves (and the list only rebuilds, + # losing that place) when the record set itself has changed under it. + async with session.stay(browser) as visit: + while True: + picked = await visit.result() + if picked is CANCEL or picked is None: # Esc + return {"records": total} + verb = picked[0] + if verb == "del_cat": + await delete_category_flow() + elif verb == "del_all": + if total and await session.typed_confirm( + f"This deletes all {total} records — every discipline, every width. " + "They can only be re-earned by walking them again.", + "delete", + title="Delete all records", + ): + ctx.repo.delete_discoveries() + else: + _verb, category, rank, record = picked + far_label, far_id, shape = walk_drawing(record) + action = await session.run_screen(RecordDialog( + record, category, rank, + resolve=resolve, device_label=device_label, device_hash=device_hash, + far_label=far_label, far_id=far_id, shape=shape, + reliability=walk_reliability(far_id, far_label), + type_of=lambda node_id: node_geo(node_id)[1], + )) + if action == "trace": + # Walking a record's path opens the Trace screen *above* the trophy + # case, like any other sub-view: Esc from the trace is one pop back + # onto the record it was armed from, and ^W is what leaves the whole + # excursion. (It used to flatten the stack first and land the reader + # on the main menu, because there was no key that could climb out of + # a deep stack in one press. Now there is.) + await open_trace_path(ctx, spec=record.spec) + elif action == "delete": + sure = await ctx.ui.dialog( + f"Delete this {category.title} record?", + [("Cancel", False), ("Delete", True)], + title="Delete record", + default=1, + destructive=True, + ) + if sure: + ctx.repo.delete_discovery(record.id) + # A trace can set a record and any of the deletes can drop several, so the + # board is rebuilt only when the record count actually moved — the one thing + # worth losing the cursor over. + if len(ctx.repo.discoveries()) != total: + break diff --git a/meshterm/ui/trace_screen.py b/meshterm/ui/trace_screen.py index d25c7a8..62a5fc9 100644 --- a/meshterm/ui/trace_screen.py +++ b/meshterm/ui/trace_screen.py @@ -121,12 +121,27 @@ _BAR_SNR_MAX = 10.0 #: runs, paced between transmissions. SAMPLE_CHOICES = (1, 2, 3, 5, 8) -#: What the trace screen resolves with when the new-record dialog's *Trophy case* -#: button is chosen. The session opener catches it and opens the trophy case only -#: after the screen has fully unwound, so leaving the trophy case afterwards lands -#: on the main menu instead of re-entering a stack of trace screens. +#: The new-record dialog's *Trophy case* button value. Choosing it opens the trophy case +#: over the trace screen, which stays pushed underneath — Esc from the trophy case is one +#: pop back onto the trace that earned the record. OPEN_TROPHY_CASE = "records" + +def _trophy_case_opener(ctx: "AppContext") -> Callable[[], Awaitable[None]]: + """A zero-argument opener for the trophy case, for the new-record dialog's left button. + + Imported inside the closure because the trophy case imports *this* module back (its + records offer a *Trace this path* of their own) — the two screens open each other, which + is exactly the cycle a strict stack allows and ^W exists to climb out of. + """ + + async def open_it() -> None: + from .records_screen import open_records + + await open_records(ctx) + + return open_it + #: A single-trace runner: ``(path_spec, on_trace)`` → runs exactly one trace, handing #: the result — plus any trophy-case disciplines the walk just placed in, as short #: ``"Title — score"`` labels — to ``on_trace`` when it lands. Provided by the session @@ -386,6 +401,7 @@ class TraceScreen(Screen): auto_spec: Callable[[], str] = lambda: "", auto_source: str = "", initial_spec: str = "", + open_trophy_case: Optional[Callable[[], Awaitable[None]]] = None, ) -> None: """Create the screen (nothing transmits until the user commits Trace). @@ -427,6 +443,9 @@ class TraceScreen(Screen): initial_spec: A forced path to open armed on, instead of idle on the auto route — how *Trace this path* from the trophy case reopens a record's exact route, ready to walk again. Empty (the default) opens on auto. + open_trophy_case: Opens the trophy case over this screen — what the new-record + dialog's left button does. ``None`` leaves the dialog with nothing but + *Close* to do, which is what a screen run outside the menu gets. """ super().__init__() self.title = f"Trace — {target}" if mode == "target" else "Trace path" @@ -449,6 +468,7 @@ class TraceScreen(Screen): self._auto_spec = auto_spec self._auto_source = auto_source self._path_spec = initial_spec.strip() + self._open_trophy_case = open_trophy_case #: The trophy-case disciplines the current run has placed in, keyed by #: discipline title so an improving multi-sample run keeps only its latest #: ``"Title — score"`` label per board. Cleared when a run starts; drained @@ -617,10 +637,13 @@ class TraceScreen(Screen): One dialog per run, however many samples scored: each placed discipline reads as its own ``★ Title — score`` line. Platform-dialog shape — *Trophy case* on the left, *Close* on the right and default — so Enter simply dismisses and Esc - closes. Choosing Trophy case resolves the whole screen with - :data:`OPEN_TROPHY_CASE`: the session opener then opens the trophy case only - *after* this screen (and everything stacked over it) has unwound, so backing - out of the trophy case lands on the main menu, never a pile of trace screens. + closes. Choosing Trophy case opens it *over* this screen, like any other + sub-view: Esc from the trophy case is one pop back onto the trace that earned + the record, and ^W is what leaves the whole excursion at once. (It used to + resolve the whole screen instead, unwinding the trace away first and landing the + reader on the main menu — a hand-built escape from a stack that had no other way + out.) The re-entrancy guard is held across the trophy case too, so a record + landing while it is open cannot stack a second dialog behind it. """ if self._dialog_open: return @@ -641,12 +664,11 @@ class TraceScreen(Screen): default=1, footer_hint="←→ choose · Enter select · Esc close", ) + if choice == OPEN_TROPHY_CASE and self._open_trophy_case is not None: + await self._open_trophy_case() finally: self._dialog_open = False self._session.invalidate() - if choice == OPEN_TROPHY_CASE: - self.cancel() - self.resolve(OPEN_TROPHY_CASE) def cancel(self) -> None: """Cancel any in-flight trace run (already-recorded traces are kept).""" @@ -2188,16 +2210,10 @@ async def _open_session( # detail screen's suggested best path handed to a target trace. In target mode it # only arms when the target is addressable — a forced path must end at a real hash. initial_spec=initial_spec if (mode == "path" or target_hash is not None) else "", + open_trophy_case=_trophy_case_opener(ctx), ) try: - result = await session.run_screen(screen) + await session.run_screen(screen) finally: screen.cancel() - if result == OPEN_TROPHY_CASE: - # The new-record dialog's hand-off: the trace screen (and every prompt over - # it) is already down, so the trophy case opens over a clean stack and Esc - # from it unwinds straight to the main menu — never back into this session. - from .records_screen import open_records - - await open_records(ctx) return screen._total_traces diff --git a/tests/test_live_screens.py b/tests/test_live_screens.py index 51821ce..51f0e04 100644 --- a/tests/test_live_screens.py +++ b/tests/test_live_screens.py @@ -206,6 +206,7 @@ def test_previous_walk_rejects_unusable_history() -> None: def _trace_screen( trace=None, compose_path=None, explore=None, pick_width=None, pick_samples=None, previous=None, mode="target", samples=1, auto_spec=None, auto_source="", + open_trophy_case=None, ) -> tuple[TraceScreen, _FakeSession]: session = _FakeSession() @@ -233,6 +234,7 @@ def _trace_screen( previous=previous, auto_spec=auto_spec or (lambda: ""), auto_source=auto_source, + open_trophy_case=open_trophy_case, ) screen.note_viewport(40) # the frame records this before every real paint return screen, session @@ -614,13 +616,25 @@ async def test_record_run_floats_the_new_record_dialog() -> None: assert screen._run_placed == {} # announced once, not again on the next run -async def test_record_dialog_trophy_case_hands_the_screen_off() -> None: - """Choosing Trophy case resolves the screen with the hand-off sentinel.""" +async def test_record_dialog_trophy_case_opens_over_the_trace_it_was_earned_on() -> None: + """Choosing Trophy case opens it *above* the trace screen, which stays up underneath. + + It used to resolve the whole screen with a sentinel so the trace unwound away first and + the reader landed on the main menu. Navigation is a strict stack now: a sub-view nests, + Esc from the trophy case is one pop back onto the trace that earned the record, and ^W is + what leaves the whole excursion. + """ + opened: list[str] = [] async def trace(path_spec, on_trace): # noqa: ANN001 on_trace(_trace(5.0), ["Most nodes — 4 nodes", "Longest distance — 12.4 km"]) - screen, session = _trace_screen(trace=trace, samples=2) + async def open_trophy_case() -> None: + opened.append("trophy case") + + screen, session = _trace_screen( + trace=trace, samples=2, open_trophy_case=open_trophy_case + ) session.dialog_answer = OPEN_TROPHY_CASE screen.future = asyncio.get_running_loop().create_future() screen.start_trace() @@ -629,7 +643,8 @@ async def test_record_dialog_trophy_case_hands_the_screen_off() -> None: _prompt, _buttons, kwargs = session.dialogs[0] assert kwargs["title"] == "2 new records" # one dialog for the whole run assert len(session.dialogs) == 1 # however many samples scored - assert screen.future.result() == OPEN_TROPHY_CASE + assert opened == ["trophy case"] + assert not screen.future.done(), "the trace screen stays up under the trophy case" async def test_escaping_mid_run_skips_the_record_dialog() -> None: diff --git a/tests/test_records_screen.py b/tests/test_records_screen.py index a97ae1f..b787c5b 100644 --- a/tests/test_records_screen.py +++ b/tests/test_records_screen.py @@ -324,8 +324,10 @@ async def test_delete_all_confirm_floats_over_the_browser(tui_ctx) -> None: """The trophy case stays drawn full-frame behind the "Delete all records" confirm. Regression: the browser was run *then popped* before the confirm opened, so the confirm - floated over a blank frame — the background "cleared". Re-pushing the browser as the - backdrop keeps it the full-frame base while the red typed-delete gate floats over it. + floated over a blank frame — the background "cleared". The browser never leaves the stack + now (``session.stay``), so it is the full-frame base while the red typed-delete gate + floats over it, and backing out returns to the very same screen — cursor included — + rather than to a rebuilt one. """ ctx = tui_ctx session = ctx.ui.session @@ -347,9 +349,9 @@ async def test_delete_all_confirm_floats_over_the_browser(tui_ctx) -> None: assert session._base_screen() is browser # the browser is still the full-frame base confirm.resolve(CANCEL) # Esc — back out without deleting - again = await _step_until(lambda: _trophy_case(session, other_than=browser)) - assert again is not None, "the browser never reopened after the confirm" - again.resolve(None) # Esc leaves the trophy case + again = await _step_until(lambda: _trophy_case(session)) + assert again is browser, "nothing was deleted, so the same screen carries on" + again.handle("escape") # Esc leaves the trophy case result = await task finally: if not task.done(): @@ -386,9 +388,9 @@ async def test_delete_a_disciplines_records_is_a_popup_over_the_browser(tui_ctx) assert session._base_screen() is browser # the browser stays behind it picker.resolve(CANCEL) # Esc — abandon the pick - again = await _step_until(lambda: _trophy_case(session, other_than=browser)) - assert again is not None - again.resolve(None) # Esc + again = await _step_until(lambda: _trophy_case(session)) + assert again is browser, "the pick was abandoned, so the same screen carries on" + again.handle("escape") # Esc result = await task finally: if not task.done(): @@ -484,14 +486,15 @@ async def test_the_board_pins_its_discipline_heading_and_description(tui_ctx) -> task.cancel() -async def test_trace_this_path_unwinds_to_the_menu_not_the_browser( +async def test_trace_this_path_nests_above_the_browser_and_comes_back_to_it( tui_ctx, monkeypatch ) -> None: - """After the Trace path hand-off, ``open_records`` returns instead of reopening. + """Trace this path opens *over* the trophy case, and Esc from it lands back on the record. - The anti-deep-stack rule: trophy case → Trace this path → (trace screen closes) - must land on the main menu, not back in the browser — otherwise bouncing between - the boards and the trace screens piles up states the user has to Esc through. + This used to flatten: the browser came down first and the whole flow returned when the + trace closed, landing the reader on the main menu — a hand-built escape from a stack that + had no other way out. Navigation is a strict stack now, so a sub-view nests like any + other and ^W is what leaves the whole excursion at once. """ ctx = tui_ctx session = ctx.ui.session @@ -503,6 +506,10 @@ async def test_trace_this_path_unwinds_to_the_menu_not_the_browser( async def fake_trace_path(ctx_, spec=""): # noqa: ANN001 walked.append(spec) + # The browser is still on the stack underneath while the trace runs. + assert any( + isinstance(s, SelectScreen) and s.title == "Trophy case" for s in session._stack + ) return 0 monkeypatch.setattr("meshterm.ui.trace_screen.open_trace_path", fake_trace_path) @@ -518,7 +525,11 @@ async def test_trace_this_path_unwinds_to_the_menu_not_the_browser( ) assert isinstance(dialog, RecordDialog) dialog.resolve("trace") # "Trace this path" - result = await task # …and the flow returns; no browser re-entry to unwind + # The trace ran and the browser is back on top — the same object, not a rebuild. + again = await _step_until(lambda: _trophy_case(session)) + assert again is browser + again.handle("escape") # only Esc leaves the trophy case + result = await task finally: if not task.done(): task.cancel()