fix(tui): convert PermissionsScreen from Static widget to proper Screen subclass
Convert PermissionsScreen from inheriting textual.widgets.Static to inheriting textual.app.Screen, making it a proper Textual Screen subclass. Added BINDINGS, action adapters for keyboard shortcuts, backwards-compatible update() override, and compose() method for widget rendering. Preserves the existing public API (load_request, clear, navigate_*, allow_*/reject_*) and unit-test semantics. ISSUES CLOSED: #10488
This commit is contained in:
@@ -8,8 +8,9 @@ Allow/reject keyboard bindings: ``a`` allow-once, ``A`` allow-always,
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import contextlib
|
||||
import importlib
|
||||
from typing import Any
|
||||
from typing import Any, ClassVar
|
||||
|
||||
from cleveragents.tui.permissions.models import (
|
||||
DiffDisplayMode,
|
||||
@@ -20,22 +21,45 @@ from cleveragents.tui.permissions.models import (
|
||||
|
||||
__all__ = ["PermissionsScreen"]
|
||||
|
||||
|
||||
# ── Optional Textual import gate ─────────────────────────────────
|
||||
|
||||
|
||||
def _load_static_base() -> type[Any]:
|
||||
"""Return a suitable base class for PermissionsScreen.
|
||||
|
||||
Prefer textual.app.Screen (proper Textual Screen subclass). If that's not
|
||||
available, fall back to textual.widgets.Static to preserve the simple
|
||||
update()/text semantics. If neither is available, provide a small
|
||||
fallback implementation used for tests and non-Textual environments.
|
||||
"""
|
||||
# Prefer the Screen class when Textual is present — this makes
|
||||
# PermissionsScreen a first-class Screen with bindings and actions.
|
||||
try:
|
||||
return importlib.import_module("textual.widgets").Static
|
||||
except Exception: # pragma: no cover
|
||||
Screen = importlib.import_module("textual.app").Screen
|
||||
return Screen
|
||||
except Exception:
|
||||
# Next, try the older Static widget (still provides update())
|
||||
try:
|
||||
return importlib.import_module("textual.widgets").Static
|
||||
except Exception: # pragma: no cover
|
||||
|
||||
class _FallbackStatic:
|
||||
def __init__(self, *args: object, **kwargs: object) -> None:
|
||||
self._text = ""
|
||||
class _FallbackStatic:
|
||||
BINDINGS: ClassVar[tuple[tuple[str, str, str], ...]] = ()
|
||||
|
||||
def update(self, text: str) -> None:
|
||||
self._text = text
|
||||
def __init__(self, *args: object, **kwargs: object) -> None:
|
||||
self._text = ""
|
||||
|
||||
return _FallbackStatic
|
||||
def update(self, text: str) -> None:
|
||||
self._text = text
|
||||
|
||||
def compose(self):
|
||||
# Keep compose() present so callers expecting a Screen API
|
||||
# can still iterate over compose() without errors.
|
||||
if False:
|
||||
yield None
|
||||
|
||||
return _FallbackStatic
|
||||
|
||||
|
||||
_StaticBase = _load_static_base()
|
||||
@@ -114,24 +138,26 @@ def _render_screen(
|
||||
|
||||
|
||||
class PermissionsScreen(_StaticBase):
|
||||
"""TUI widget that displays a tool permission request with a diff view.
|
||||
"""TUI screen that displays a tool permission request with a diff view.
|
||||
|
||||
Layout:
|
||||
- Title: "Permission Request"
|
||||
- Left panel: file list with change type indicators
|
||||
- Right panel: diff view for the selected file
|
||||
- Status bar: keyboard bindings and diff mode indicator
|
||||
|
||||
Keyboard bindings:
|
||||
- ``a``: allow once
|
||||
- ``A``: allow always (session)
|
||||
- ``r``: reject once
|
||||
- ``R``: reject always (session)
|
||||
- ``j`` / ``k``: navigate file list (next / previous)
|
||||
- ``d``: cycle diff display mode (unified → side-by-side → context)
|
||||
- ``escape``: dismiss (caller responsibility)
|
||||
This class intentionally maintains the simple text/update API used by
|
||||
unit tests while offering a proper Textual Screen when Textual is
|
||||
available. The public API (load_request, clear, navigate_*, allow_*/reject_*)
|
||||
remains the same.
|
||||
"""
|
||||
|
||||
# Keyboard bindings: 8 shortcuts as required by the TUI design
|
||||
BINDINGS: ClassVar[tuple[tuple[str, str, str], ...]] = (
|
||||
("a", "allow_once", "Allow once"),
|
||||
("A", "allow_always", "Allow always"),
|
||||
("r", "reject_once", "Reject once"),
|
||||
("R", "reject_always", "Reject always"),
|
||||
("j", "navigate_next", "Next file"),
|
||||
("k", "navigate_prev", "Previous file"),
|
||||
("d", "cycle_diff_mode", "Cycle diff mode"),
|
||||
("escape", "dismiss", "Dismiss"),
|
||||
)
|
||||
|
||||
def __init__(self, *args: object, **kwargs: object) -> None:
|
||||
super().__init__(*args, **kwargs)
|
||||
self._request: ToolPermissionRequest | None = None
|
||||
@@ -139,6 +165,7 @@ class PermissionsScreen(_StaticBase):
|
||||
self._diff_mode: DiffDisplayMode = DiffDisplayMode.UNIFIED
|
||||
self._decision: PermissionDecision | None = None
|
||||
self._text: str = "(no permission request)"
|
||||
# Use our own update method which will call the base's update if present
|
||||
self.update(self._text)
|
||||
|
||||
# ── Public API ────────────────────────────────────────────────
|
||||
@@ -238,6 +265,36 @@ class PermissionsScreen(_StaticBase):
|
||||
self._refresh()
|
||||
return decision
|
||||
|
||||
# ── Textual action adapters (so key bindings call the same API) ───
|
||||
|
||||
# The Textual framework maps BINDINGS entries to methods named
|
||||
# `action_<name>()`. Provide these adapters to keep the public API
|
||||
# identical whether running as a Screen or as a fallback widget.
|
||||
|
||||
def action_allow_once(self) -> None: # pragma: no cover - exercised in UI only
|
||||
self.allow_once()
|
||||
|
||||
def action_allow_always(self) -> None: # pragma: no cover - exercised in UI only
|
||||
self.allow_always()
|
||||
|
||||
def action_reject_once(self) -> None: # pragma: no cover - exercised in UI only
|
||||
self.reject_once()
|
||||
|
||||
def action_reject_always(self) -> None: # pragma: no cover - exercised in UI only
|
||||
self.reject_always()
|
||||
|
||||
def action_navigate_next(self) -> None: # pragma: no cover - exercised in UI only
|
||||
self.navigate_next()
|
||||
|
||||
def action_navigate_prev(self) -> None: # pragma: no cover - exercised in UI only
|
||||
self.navigate_prev()
|
||||
|
||||
def action_cycle_diff_mode(self) -> None: # pragma: no cover - exercised in UI only
|
||||
self.cycle_diff_mode()
|
||||
|
||||
def action_dismiss(self) -> None: # pragma: no cover - exercised in UI only
|
||||
self.clear()
|
||||
|
||||
# ── Rendering ─────────────────────────────────────────────────
|
||||
|
||||
def _refresh(self) -> None:
|
||||
@@ -252,3 +309,42 @@ class PermissionsScreen(_StaticBase):
|
||||
self._diff_mode,
|
||||
)
|
||||
self.update(self._text)
|
||||
|
||||
# ── Backwards-compatible update() for Static and Screen bases ───
|
||||
|
||||
def update(self, text: str) -> None:
|
||||
"""Update the stored text and, if the base class exposes a working
|
||||
`update()` method, delegate to it so Textual widgets remain functional.
|
||||
"""
|
||||
self._text = text
|
||||
# If the underlying base class has an `update` implementation (e.g.
|
||||
# textual.widgets.Static), call it. For textual.app.Screen there is no
|
||||
# `update()` to call, so this safely no-ops.
|
||||
super_update = getattr(super(), "update", None)
|
||||
if callable(super_update):
|
||||
# Best-effort delegation to the base class update implementation.
|
||||
with contextlib.suppress(Exception):
|
||||
super_update(text)
|
||||
|
||||
def compose(self): # pragma: no cover - UI-only helper
|
||||
"""Compose the child widgets when running under Textual.
|
||||
|
||||
This function yields at least one widget so Textual's compose() loop
|
||||
can mount something useful during real UI operation. In non-Textual
|
||||
environments it yields a single None to remain a generator.
|
||||
"""
|
||||
try:
|
||||
module = importlib.import_module("textual.widgets")
|
||||
Static = getattr(module, "Static", None)
|
||||
if Static is not None:
|
||||
yield Static(self._text)
|
||||
return
|
||||
except Exception:
|
||||
# Non-Textual environments or import failure fall through to a
|
||||
# generator that yields nothing meaningful (keeps API stable).
|
||||
pass
|
||||
|
||||
# Ensure this function is a generator even when Textual isn't present.
|
||||
if False:
|
||||
yield None
|
||||
|
||||
|
||||
Reference in New Issue
Block a user