Merge branch 'new-session-dialog-ux'
New Session dialog UX refinements: reveal sandbox sub-options only when sandboxing is on, unify focus highlight to the Peach accent, and add soft j/k field navigation.
This commit is contained in:
@@ -1,4 +1,5 @@
|
||||
from textual.app import ComposeResult
|
||||
from textual.binding import Binding
|
||||
from textual.containers import Horizontal, Vertical
|
||||
from textual.screen import ModalScreen
|
||||
from textual.widgets import Button, Checkbox, Input, Label, Select, Switch
|
||||
@@ -10,6 +11,14 @@ from hqt.sandbox import SandboxPolicy
|
||||
class NewSessionScreen(
|
||||
ModalScreen[tuple[str, str, str | None, str | None, SandboxPolicy | None] | None]
|
||||
):
|
||||
# Soft vim-style field navigation: j/k walk the dialog's focus chain. Inputs
|
||||
# consume printable keys first, so j/k type literally while editing text and
|
||||
# only move focus when a toggle/select/button is focused.
|
||||
BINDINGS = [
|
||||
Binding("j", "focus_next", "Next field", show=False),
|
||||
Binding("k", "focus_previous", "Prev field", show=False),
|
||||
]
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
harness_names: list[str],
|
||||
@@ -21,6 +30,14 @@ class NewSessionScreen(
|
||||
self.sandbox_available = sandbox_available
|
||||
super().__init__()
|
||||
|
||||
# focus_next/focus_previous are screen methods, not binding actions, so the
|
||||
# j/k bindings need these thin action_* wrappers to dispatch to them.
|
||||
def action_focus_next(self) -> None:
|
||||
self.focus_next()
|
||||
|
||||
def action_focus_previous(self) -> None:
|
||||
self.focus_previous()
|
||||
|
||||
def compose(self) -> ComposeResult:
|
||||
with Vertical(id="new-session-dialog"):
|
||||
yield Label("New Session")
|
||||
@@ -63,15 +80,20 @@ class NewSessionScreen(
|
||||
"bubblewrap not found — run `hqt doctor`",
|
||||
id="sandbox-warning",
|
||||
)
|
||||
yield Label("Filesystem access:")
|
||||
yield Select(
|
||||
[("Read-write", "rw"), ("Read-only", "ro")],
|
||||
id="fs-select",
|
||||
value="rw",
|
||||
allow_blank=False,
|
||||
)
|
||||
yield Label("Network:")
|
||||
yield Switch(id="net-switch", value=True)
|
||||
# Sub-options of the sandbox switch: only meaningful when sandboxing
|
||||
# is on, so hidden until then (mirrors the worktree -> branch reveal).
|
||||
sandbox_options = Vertical(id="sandbox-options")
|
||||
sandbox_options.display = False
|
||||
with sandbox_options:
|
||||
yield Label("Filesystem access:")
|
||||
yield Select(
|
||||
[("Read-write", "rw"), ("Read-only", "ro")],
|
||||
id="fs-select",
|
||||
value="rw",
|
||||
allow_blank=False,
|
||||
)
|
||||
yield Label("Network:")
|
||||
yield Switch(id="net-switch", value=True)
|
||||
with Horizontal(classes="dialog-actions"):
|
||||
yield Button("OK", variant="primary", id="ok-btn")
|
||||
yield Button("Cancel", id="cancel-btn")
|
||||
@@ -89,6 +111,11 @@ class NewSessionScreen(
|
||||
else:
|
||||
branch_input.display = False
|
||||
|
||||
def on_switch_changed(self, event: Switch.Changed) -> None:
|
||||
if event.switch.id != "sandbox-switch":
|
||||
return
|
||||
self.query_one("#sandbox-options", Vertical).display = event.value
|
||||
|
||||
def on_button_pressed(self, event: Button.Pressed) -> None:
|
||||
if event.button.id == "ok-btn":
|
||||
self._submit()
|
||||
|
||||
@@ -90,3 +90,15 @@ ProjectFormScreen, NewSessionScreen, SchedulePromptScreen {
|
||||
background: $surface-lighten-1;
|
||||
color: $foreground;
|
||||
}
|
||||
|
||||
/* Unify the focus highlight across the New Session dialog. The buttons get a
|
||||
Peach $accent focus highlight (above), but Switch/Checkbox/Select/Input
|
||||
otherwise fall back to Textual's default Lavender $border focus border, which
|
||||
reads as off-palette next to the buttons. Match `tall` so only the color
|
||||
changes, not the control height. */
|
||||
#new-session-dialog Switch:focus,
|
||||
#new-session-dialog Checkbox:focus,
|
||||
#new-session-dialog Select:focus,
|
||||
#new-session-dialog Input:focus {
|
||||
border: tall $accent;
|
||||
}
|
||||
|
||||
@@ -1869,3 +1869,117 @@ async def test_poll_sessions_dispatches_due_prompts_and_notifies():
|
||||
|
||||
mock_service.dispatch_due_prompts.assert_awaited_once()
|
||||
assert any("Prompt sent" in call[0] for call in notify_calls)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sandbox_options_hidden_until_switch_on():
|
||||
"""The fs/net rows are sub-options of the sandbox switch: hidden until it is
|
||||
turned on, shown when on, hidden again when turned back off."""
|
||||
app = HqtApp()
|
||||
async with app.run_test(size=(120, 40)) as pilot:
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
from textual.widgets import Switch
|
||||
|
||||
app.push_screen(
|
||||
NewSessionScreen(["claude"], is_git_repo=True, sandbox_available=True)
|
||||
)
|
||||
await pilot.pause()
|
||||
options = app.screen.query_one("#sandbox-options")
|
||||
assert options.display is False
|
||||
|
||||
switch = app.screen.query_one("#sandbox-switch", Switch)
|
||||
switch.value = True
|
||||
await pilot.pause()
|
||||
assert options.display is True
|
||||
|
||||
switch.value = False
|
||||
await pilot.pause()
|
||||
assert options.display is False
|
||||
|
||||
|
||||
def test_dialog_focus_highlight_uses_accent():
|
||||
"""Every focusable control in the New Session dialog shares the Peach accent
|
||||
focus border (consistent with the dialog buttons), not Textual's default
|
||||
Lavender. Guards against the scoped :focus rule being dropped."""
|
||||
from pathlib import Path
|
||||
import hqt.tui
|
||||
|
||||
css = (Path(hqt.tui.__file__).parent / "styles.tcss").read_text()
|
||||
for selector in (
|
||||
"#new-session-dialog Switch:focus",
|
||||
"#new-session-dialog Checkbox:focus",
|
||||
"#new-session-dialog Select:focus",
|
||||
"#new-session-dialog Input:focus",
|
||||
):
|
||||
assert selector in css, f"missing focus rule: {selector}"
|
||||
assert "border: tall $accent;" in css
|
||||
|
||||
|
||||
def test_new_session_has_jk_focus_bindings():
|
||||
"""j -> focus_next, k -> focus_previous, declared on the screen."""
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
|
||||
actions = {b.key: b.action for b in NewSessionScreen.BINDINGS}
|
||||
assert actions.get("j") == "focus_next"
|
||||
assert actions.get("k") == "focus_previous"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_jk_moves_focus_between_controls():
|
||||
"""On a non-text control, j moves focus to another control."""
|
||||
app = HqtApp()
|
||||
async with app.run_test(size=(120, 40)) as pilot:
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
from textual.widgets import Switch
|
||||
|
||||
app.push_screen(
|
||||
NewSessionScreen(["claude"], is_git_repo=True, sandbox_available=True)
|
||||
)
|
||||
await pilot.pause()
|
||||
switch = app.screen.query_one("#sandbox-switch", Switch)
|
||||
switch.focus()
|
||||
await pilot.pause()
|
||||
assert app.focused is switch
|
||||
|
||||
await pilot.press("j")
|
||||
await pilot.pause()
|
||||
assert app.focused is not switch
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_jk_typed_into_input_is_literal():
|
||||
"""Inside a text input, j/k type literally and do not move focus."""
|
||||
app = HqtApp()
|
||||
async with app.run_test(size=(120, 40)) as pilot:
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
from textual.widgets import Input
|
||||
|
||||
app.push_screen(NewSessionScreen(["claude"], is_git_repo=True))
|
||||
await pilot.pause()
|
||||
nickname = app.screen.query_one("#nickname-input", Input)
|
||||
nickname.focus()
|
||||
await pilot.pause()
|
||||
|
||||
await pilot.press("j", "k")
|
||||
await pilot.pause()
|
||||
assert nickname.value == "jk"
|
||||
assert app.focused is nickname
|
||||
|
||||
|
||||
def test_policy_is_none_when_sandbox_off_regardless_of_suboptions():
|
||||
"""When sandboxing is off, the (now hidden) fs/net sub-option values are
|
||||
discarded — the submitted policy is None no matter what they hold."""
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
|
||||
assert NewSessionScreen._policy_from(False, "ro", False) is None
|
||||
assert NewSessionScreen._policy_from(False, "rw", True) is None
|
||||
|
||||
|
||||
def test_policy_reflects_suboptions_when_sandbox_on():
|
||||
"""When sandboxing is on, the fs/net sub-option values flow into the policy."""
|
||||
from hqt.tui.screens.new_session import NewSessionScreen
|
||||
|
||||
policy = NewSessionScreen._policy_from(True, "ro", False)
|
||||
assert policy is not None
|
||||
assert policy.fs == "ro"
|
||||
assert policy.net is False
|
||||
|
||||
Reference in New Issue
Block a user