From a8b69efa73218406c998e7766ad237ef9ca0199c Mon Sep 17 00:00:00 2001 From: Jean-Pierre Martineau Date: Sat, 12 Sep 2026 02:05:21 -0400 Subject: [PATCH] The repeater admin's node picker is a popup, gone before the admin page opens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The same shape TX optimize just lost, with one popup instead of two: the node list was kept pushed as a hub under the password prompt and the admin page, so Esc from the page landed on the list it was picked from β€” two places where there is one. A popup asks and is gone: the pick is now a floating one-shot (session.select gains text's floating flag, so a lead-in question draws as a box even as the only frame on the stack), the admin page opens over the main menu, and a refused login re-asks the question with the same node highlighted. admin_node_visit is deleted; nothing keeps a popup pushed now. Co-Authored-By: Claude Fable 5.1 --- docs/survey-navigation.md | 7 +++ meshterm/ui/admin_picker.py | 89 +++++++++-------------------------- meshterm/ui/repeater_admin.py | 51 +++++++++----------- meshterm/ui/surface.py | 7 ++- meshterm/ui/tui/session.py | 9 +++- tests/test_repeater_admin.py | 48 +++++++++---------- 6 files changed, 89 insertions(+), 122 deletions(-) diff --git a/docs/survey-navigation.md b/docs/survey-navigation.md index c47be86..3cfd5f9 100644 --- a/docs/survey-navigation.md +++ b/docs/survey-navigation.md @@ -87,6 +87,13 @@ of the two pickers never looked like a question at all. `stay(dialog=True)` now blank base under a popup's visit that `_run_dialog_screen` puts under a one-shot. Trace's picker is a full-screen contact list (a place), and stays as it is. +*Same day β€” Repeater admin.* The same shape, one popup instead of two: the node picker was +a hub under the password prompt and the admin page. It is now a floating one-shot +(`session.select(floating=True)`, `admin_picker.pick_admin_node`), gone once answered; the +admin page opens over the main menu and Esc from it lands there. A refused login re-asks +the question with the same node highlighted. `admin_node_visit` is deleted β€” nothing keeps +a popup pushed any more. + **I. An entry chain of prompts was not a stack.** Five flows asked two or more things in a row, and Esc on any of them abandoned the whole flow rather than stepping back one β€” a mistyped 32-hex channel key cost the name typed before it. `menus.run_steps` now runs a diff --git a/meshterm/ui/admin_picker.py b/meshterm/ui/admin_picker.py index 0dbb01a..4cb1f13 100644 --- a/meshterm/ui/admin_picker.py +++ b/meshterm/ui/admin_picker.py @@ -6,12 +6,15 @@ Credentialed nodes lead the list in their own πŸ”‘ section (those are the nodes administered, the likeliest picks), repeaters and room servers follow, then everything else; each section orders by how recently the node was heard. Any contact with a public key is offerable, since holding a password is a fact about the *user*, not the node. + +The pick is a **popup**: a question asked on the way into a tool, drawn as a box and gone +the moment it is answered β€” never a frame kept under the tool's own page for Esc to land +back on. It used to be held pushed as a hub for the whole visit, so leaving the admin page +landed on the list it was picked from; that read as two places where there is one. """ from __future__ import annotations -from collections.abc import AsyncIterator -from contextlib import asynccontextmanager from typing import TYPE_CHECKING, Any from rich.text import Text @@ -25,11 +28,10 @@ if TYPE_CHECKING: def admin_picker_rows(ctx: AppContext, contacts: list[Contact]) -> tuple[list, list[Contact]]: """Build the admin-node picker's grouped rows and the contacts they map to. - Shared so the pick and any later redraw are guaranteed identical: :func:`pick_admin_node` - runs these rows, and a caller can rebuild the same list as a static backdrop to float a - follow-up dialog (the admin login) over the very list the node was picked from. Rows carry - ``value = contact.name``, so a picked name (or a backdrop's ``default``) resolves through - ``candidates``. + Shared so every surface that offers the pick draws the same list: :func:`pick_admin_node` + runs these rows as a one-shot popup, and TX optimize turns them as the first page of its + stepped dialog. Rows carry ``value = contact.name``, so a picked name (or a ``default``) + resolves through ``candidates``. Args: ctx: Shared application context (for the admin store, which sorts remembered nodes up). @@ -92,18 +94,21 @@ async def pick_admin_node( *, title: str, prompt: str, + default: Contact | None = None, ) -> Contact | None: """Pick a remote node to administer, credentialed and infrastructure nodes first. - The one-shot form: the list comes down as soon as a node is picked. A caller whose next - step belongs *over* the list β€” the admin login, which asks about the node just picked β€” - wants :func:`admin_node_visit` instead. + A floating popup that comes down as soon as a node is picked. A caller that asks again + β€” the login it tried was refused β€” hands the last pick back as ``default`` so the list + reopens on it, the way a step of :func:`~meshterm.ui.menus.run_steps` offers its + previous answer. Args: ctx: Shared application context (for the UI surface and the admin store). contacts: The device's known contacts. title: The select screen's heading (names the calling feature). prompt: One line above the list saying what the pick is for. + default: The node to open highlighted on, if any. Returns: The chosen contact, or ``None`` if cancelled (or there is nothing to pick). @@ -113,66 +118,16 @@ async def pick_admin_node( await _note_nothing_to_pick(ctx, title) return None - choice = await ctx.ui.select(title, items, prompt=prompt) + choice = await ctx.ui.select( + title, + items, + prompt=prompt, + default=default.name if default is not None else None, + floating=True, + ) return _resolve(candidates, choice) -@asynccontextmanager -async def admin_node_visit( - ctx: AppContext, - contacts: list[Contact], - *, - title: str, - prompt: str, -) -> AsyncIterator[_AdminNodePicker | None]: - """Keep the node picker pushed for a whole visit, yielding a picker to draw from. - - The list stays on the stack for the duration of the block, so everything the caller does - with a picked node β€” the password prompt, a rejection, the admin session itself β€” nests - *above* the very list the pick came from, with the picked node still highlighted, instead - of floating over a blank frame or being drawn again as a lookalike backdrop. Esc from any - of it is one pop back onto the list. - - Yields ``None`` when there is nothing to pick (the caller has already been told). - - Args: - ctx: Shared application context (for the UI surface and the admin store). - contacts: The device's known contacts. - title: The select screen's heading (names the calling feature). - prompt: One line above the list saying what the pick is for. - """ - from .surface import TuiUi - from .tui import SelectScreen - - items, candidates = admin_picker_rows(ctx, contacts) - if not candidates or not isinstance(ctx.ui, TuiUi): - if not candidates: - await _note_nothing_to_pick(ctx, title) - yield None - return - screen = SelectScreen(title, items, prompt=prompt) - async with ctx.ui.session.stay(screen) as visit: - yield _AdminNodePicker(visit, candidates) - - -class _AdminNodePicker: - """The visited node list: :meth:`pick` is one round of it, resolved to a contact.""" - - __slots__ = ("_visit", "_candidates") - - def __init__(self, visit: Any, candidates: list[Contact]) -> None: - """Bind the picker to the list's visit and the contacts its rows stand for.""" - self._visit = visit - self._candidates = candidates - - async def pick(self) -> Contact | None: - """Await one pick: the chosen contact, or ``None`` on Esc.""" - from .tui.screen import CANCEL - - choice = await self._visit.result() - return _resolve(self._candidates, None if choice is CANCEL else choice) - - async def _note_nothing_to_pick(ctx: AppContext, title: str) -> None: """Say there is no offerable node and show it β€” no list is opened at all.""" ctx.ui.note("[err]no contacts with a key β€” receive an advert first[/err]") diff --git a/meshterm/ui/repeater_admin.py b/meshterm/ui/repeater_admin.py index dfd5abf..e55d061 100644 --- a/meshterm/ui/repeater_admin.py +++ b/meshterm/ui/repeater_admin.py @@ -174,21 +174,24 @@ class AdminMenu(SelectScreen): async def open_repeater_admin(ctx: AppContext) -> dict[str, Any] | None: """Run the repeater-admin flow: pick a node, log in, and administer it. - The picker stays up for the whole flow, so leaving a node's admin session lands back on - the list rather than on the main menu β€” administering two repeaters in a row is one Esc - and a pick, not a round trip through the menu. + The pick is a popup β€” a question on the way in, gone once answered β€” so the admin page + opens over the main menu and Esc from it lands there: the page is the whole visit. A + login the node refuses (or never answers) re-asks the question with the same node + highlighted, so a mistyped password is one Enter and a retry, not a round trip. The + list used to stay pushed under the page as a hub, which read as two places where + there is one. Args: ctx: The shared application context (must be running the interactive TUI surface). Returns: - A summary of the last node administered (for the tool's log), or ``None`` if the - reader left the picker without ever getting into a session. + A summary of the node administered (for the tool's log), or ``None`` if the reader + left the picker without ever getting into a session. Raises: RuntimeError: If called outside the interactive menu (no full-screen session). """ - from .admin_picker import admin_node_visit + from .admin_picker import pick_admin_node from .surface import TuiUi if not isinstance(ctx.ui, TuiUi): # pragma: no cover - guarded by the menu-only caller @@ -200,27 +203,19 @@ async def open_repeater_admin(ctx: AppContext) -> dict[str, Any] | None: # :class:`~meshterm.services.device_state.DeviceState`); the device handle below is still # needed for the admin login and the CLI session that follow. contacts = await ctx.devstate.contacts() - # The node list stays pushed for the whole flow, so the password prompt (and any - # rejection) floats over the very list the pick came from, and the admin session opens - # above it: Esc out of the session lands back on the list, ready to manage another node, - # and Esc again leaves. It used to draw a second, identical SelectScreen as a static - # backdrop for the login and then drop the list entirely before the session. - async with admin_node_visit( - ctx, - contacts, - title="Repeater admin β€” node to manage", - prompt="The remote node to set up (you need its admin password):", - ) as picker: - if picker is None: # nothing offerable; the reader has been told + node: Contact | None = None + while True: + node = await pick_admin_node( + ctx, + contacts, + title="Repeater admin β€” node to manage", + prompt="The remote node to set up (you need its admin password):", + default=node, + ) + if node is None: return None - summary: dict[str, Any] | None = None - while True: - node = await picker.pick() - if node is None: - return summary - if not await _login(ctx, device, node): - continue - summary = await _admin_session(ctx, device, node) + if await _login(ctx, device, node): + return await _admin_session(ctx, device, node) async def _login(ctx: AppContext, device: Device, node: Contact) -> bool: @@ -241,8 +236,8 @@ async def _login(ctx: AppContext, device: Device, node: Contact) -> bool: f"Admin password for {node.name}", prompt="The node ignores admin commands without a login.", password=True, - # Floats over the picker backdrop the caller keeps pushed; the flag is the - # belt-and-suspenders so it never fills the frame even if that backdrop is absent. + # The picker popup is gone by now, so this is the only frame: the flag is what + # keeps it a box over a blank base rather than a full-frame prompt. floating=True, ) if not password: diff --git a/meshterm/ui/surface.py b/meshterm/ui/surface.py index 08a4201..4002444 100644 --- a/meshterm/ui/surface.py +++ b/meshterm/ui/surface.py @@ -163,13 +163,15 @@ class Ui: default: Any = None, filterable: bool = True, delete_hint: str = "", + floating: bool = False, ) -> Any: """Prompt the user to choose one item; return its value or ``None`` if cancelled. ``prompt`` draws an instruction inside the popup, above the list. Pass ``filterable=False`` for a short, fixed list so a stray key can't narrow (and resize) it. ``delete_hint`` (with rows marked deletable) enables the Delete - key's remove flow β€” see :meth:`~meshterm.ui.tui.session.TuiSession.select`. + key's remove flow, and ``floating`` keeps a lead-in question drawn as a box even + with nothing under it β€” see :meth:`~meshterm.ui.tui.session.TuiSession.select`. """ raise NotImplementedError @@ -458,6 +460,7 @@ class PlainUi(Ui): default: Any = None, filterable: bool = True, delete_hint: str = "", + floating: bool = False, ) -> Any: """Unsupported in scripted CLI mode.""" raise self._no_prompt() @@ -712,6 +715,7 @@ class TuiUi(Ui): default: Any = None, filterable: bool = True, delete_hint: str = "", + floating: bool = False, ) -> Any: """Delegate to the session's select screen.""" return await self.session.select( @@ -721,6 +725,7 @@ class TuiUi(Ui): default=default, filterable=filterable, delete_hint=delete_hint, + floating=floating, ) async def select_startup( diff --git a/meshterm/ui/tui/session.py b/meshterm/ui/tui/session.py index c47e67f..5eed34b 100644 --- a/meshterm/ui/tui/session.py +++ b/meshterm/ui/tui/session.py @@ -871,6 +871,7 @@ class TuiSession: filterable: bool = True, footer_hint: str | None = None, delete_hint: str = "", + floating: bool = False, ) -> Any: """Show a select screen; return the chosen value or ``None`` if cancelled. @@ -879,7 +880,10 @@ class TuiSession: want no type-to-filter and a tailored hint. ``delete_hint`` (with rows marked :attr:`~meshterm.ui.tui.select.Choice.deletable`) surfaces the Delete key's atom while the highlight sits on such a row; Delete then resolves a - :class:`~meshterm.ui.tui.select.DeleteRequest` the caller unwraps. + :class:`~meshterm.ui.tui.select.DeleteRequest` the caller unwraps. ``floating`` + is :meth:`text`'s: the list is a *question* asked on the way into a tool (which + node to administer) rather than the tool's own page, so it must draw as a box + even when it is the only frame on the stack β€” see :meth:`_floated`. """ kwargs: dict[str, Any] = dict( prompt=prompt, @@ -889,7 +893,8 @@ class TuiSession: ) if footer_hint is not None: kwargs["footer_hint"] = footer_hint - result = await self.run_screen(SelectScreen(title, items, **kwargs)) + runner = self._run_dialog_screen if floating else self.run_screen + result = await runner(SelectScreen(title, items, **kwargs)) return None if result is CANCEL else result async def select_startup( diff --git a/tests/test_repeater_admin.py b/tests/test_repeater_admin.py index 5ce722c..51d2e9c 100644 --- a/tests/test_repeater_admin.py +++ b/tests/test_repeater_admin.py @@ -387,30 +387,30 @@ def test_picker_rows_hue_each_name_by_its_own_key(tui_ctx) -> None: assert len(hues) == 2 # two repeaters, two identities β€” not one violet block -async def test_login_password_floats_over_the_node_picker(tui_ctx) -> None: - """The admin-login password prompt floats over the picker itself, not a lookalike. +async def test_the_node_picker_is_a_popup_gone_before_the_login(tui_ctx) -> None: + """The node pick is a floating question, answered and gone; a refused login re-asks it. - Regression: ``_login`` ran on an empty stack (the picker was popped when it returned), - so ``session.text`` pushed a *blank* base and the password box floated over an erased - frame. That was patched by redrawing a second, identical ``SelectScreen`` as a static - backdrop; the picker now simply never leaves the stack, so the base under the prompt is - the list the pick came from β€” the same object, still highlighted on the row the reader - chose β€” and Esc out of the prompt is one pop back onto it rather than out of the flow. + The picker used to stay pushed as a hub under the password prompt and the admin page, + so Esc from the page landed on the list it was picked from β€” two places where there + is one. Now it draws as a box over a blank base (a popup that is the only frame on the + stack would otherwise be painted full-frame), comes down on Enter, the password prompt + floats on its own, and Esc out of the prompt asks the question again with the same node + highlighted rather than dropping the reader on a list that never left. """ ctx = tui_ctx session = ctx.ui.session pick_title = "Repeater admin β€” node to manage" + def picker_up() -> SelectScreen | None: + top = session.top + return top if isinstance(top, SelectScreen) and top.title == pick_title else None + task = asyncio.ensure_future(open_repeater_admin(ctx)) try: - picker = await _step_until( - lambda: ( - session.top - if isinstance(session.top, SelectScreen) and session.top.title == pick_title - else None - ) - ) + picker = await _step_until(picker_up) assert picker is not None, "the node picker never opened" + assert picker.floating and session._has_float(), "drawn as a box…" + assert not session._base_screen().floating, "…over a blank base" # Walk to the repeater's row and commit it, the way a reader does. for _ in range(len(picker._items)): @@ -420,24 +420,24 @@ async def test_login_password_floats_over_the_node_picker(tui_ctx) -> None: picker.handle("down") picker.handle("enter") prompt = await _step_until( - lambda: session._float_layers()[0] if session._has_float() else None + lambda: session.top if isinstance(session.top, TextScreen) else None ) - assert isinstance(prompt, TextScreen) # the password box floats - - assert session._base_screen() is picker # the very list, not a redrawn stand-in - current = picker._current_choice() # still highlighted on the node being logged into - assert current is not None and current.value == "Yagi-Repeater" + assert prompt.floating and session._has_float(), "the password box floats" + assert picker not in session._stack, "the pick is answered, so the popup is gone" prompt.resolve(CANCEL) # Esc β€” abandon the login… - again = await _step_until(lambda: session.top if session.top is picker else None) - assert again is picker, "…and land back on the list, one pop, place kept" - picker.handle("escape") # Esc again leaves the flow + again = await _step_until(picker_up) + assert again is not picker, "…and the question is asked again, a fresh popup" + current = again._current_choice() + assert current is not None and current.value == "Yagi-Repeater", "same node highlighted" + again.handle("escape") # Esc on the question leaves the flow result = await task finally: if not task.done(): task.cancel() assert result is None # no node was ever administered + assert session._stack == [] # --- reading and applying against the simulator ----------------------------------------