docs: implementation plan for New Session dialog UX refinements
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,353 @@
|
||||
# New Session Dialog UX Refinements Implementation Plan
|
||||
|
||||
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
||||
|
||||
**Goal:** Three UX refinements to the New Session modal — hide sandbox sub-options until sandboxing is enabled, unify the focus highlight to the dialog's Peach accent, and add context-aware j/k field navigation.
|
||||
|
||||
**Architecture:** All changes are confined to the `NewSessionScreen` widget (`src/hqt/tui/screens/new_session.py`) and the shared stylesheet (`src/hqt/tui/styles.tcss`). The reveal logic reuses the screen's existing show/hide pattern (the worktree checkbox → branch input), navigation uses Textual's built-in `focus_next`/`focus_previous` screen actions, and the highlight is a scoped CSS rule. No service, DB, or sandbox-policy code is touched.
|
||||
|
||||
**Tech Stack:** Python 3.14, Textual (TUI framework), pytest + pytest-asyncio (`app.run_test()` pilot harness), uv for running commands.
|
||||
|
||||
---
|
||||
|
||||
## Conventions
|
||||
|
||||
- Run tests with: `uv run --active pytest <path> -v`
|
||||
- Run the linter with: `uv run --active ruff check`
|
||||
- Existing screen tests live in `tests/test_tui.py` and use the `app.run_test()` pilot. New tests go there too.
|
||||
- The screen under test is constructed as `NewSessionScreen(harness_names, is_git_repo=..., sandbox_available=...)` and pushed with `app.push_screen(...)`.
|
||||
- Commit messages end with the trailer:
|
||||
`Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>`
|
||||
|
||||
## File Structure
|
||||
|
||||
- `src/hqt/tui/screens/new_session.py` — modal screen. Gains: a `#sandbox-options` container around the fs/net rows, an `on_switch_changed` handler, and `BINDINGS` for j/k.
|
||||
- `src/hqt/tui/styles.tcss` — shared stylesheet. Gains: a scoped `:focus` rule for the dialog's controls.
|
||||
- `tests/test_tui.py` — screen tests. Gains: reveal, focus-highlight, and j/k navigation tests.
|
||||
|
||||
---
|
||||
|
||||
### Task 1: Hide sandbox sub-options until the switch is on
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/hqt/tui/screens/new_session.py:66-74` (the fs/net rows) and add an `on_switch_changed` handler near `on_checkbox_changed` (around line 79)
|
||||
- Test: `tests/test_tui.py`
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Append to `tests/test_tui.py`:
|
||||
|
||||
```python
|
||||
@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
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py::test_sandbox_options_hidden_until_switch_on -v`
|
||||
Expected: FAIL — `NoMatches` / no widget with id `sandbox-options` (the container does not exist yet).
|
||||
|
||||
- [ ] **Step 3: Wrap the fs/net rows in a hidden container**
|
||||
|
||||
In `src/hqt/tui/screens/new_session.py`, replace the current fs/net block (lines 66-74):
|
||||
|
||||
```python
|
||||
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 a container that starts hidden (same `display = False` pattern already used
|
||||
for `#branch-input` above):
|
||||
|
||||
```python
|
||||
# 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)
|
||||
```
|
||||
|
||||
`Vertical` is already imported at the top of the file (`from textual.containers import Horizontal, Vertical`).
|
||||
|
||||
- [ ] **Step 4: Add the reveal handler**
|
||||
|
||||
In the same file, add this method directly after `on_checkbox_changed` (it ends
|
||||
around line 90). Guard on the switch id so the `#net-switch` toggle inside the
|
||||
container does not trigger it:
|
||||
|
||||
```python
|
||||
def on_switch_changed(self, event: Switch.Changed) -> None:
|
||||
if event.switch.id != "sandbox-switch":
|
||||
return
|
||||
self.query_one("#sandbox-options").display = event.value
|
||||
```
|
||||
|
||||
- [ ] **Step 5: Run test to verify it passes**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py::test_sandbox_options_hidden_until_switch_on -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/hqt/tui/screens/new_session.py tests/test_tui.py
|
||||
git commit -m "feat: reveal sandbox sub-options only when sandboxing is on
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 2: Unify the focus highlight to the Peach accent
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/hqt/tui/styles.tcss` (append a scoped `:focus` rule)
|
||||
- Test: `tests/test_tui.py`
|
||||
|
||||
Context: the dialog buttons already get a Peach `$accent` focus highlight, but
|
||||
`Switch`/`Checkbox`/`Select`/`Input` fall back to Textual's default Lavender
|
||||
(`$border`) focus border. This task makes every focusable control in the dialog
|
||||
share the Peach accent.
|
||||
|
||||
- [ ] **Step 1: Write the failing test**
|
||||
|
||||
Append to `tests/test_tui.py`:
|
||||
|
||||
```python
|
||||
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
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run test to verify it fails**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py::test_dialog_focus_highlight_uses_accent -v`
|
||||
Expected: FAIL — selectors not found in `styles.tcss`.
|
||||
|
||||
- [ ] **Step 3: Add the scoped focus rule**
|
||||
|
||||
Append to the end of `src/hqt/tui/styles.tcss`:
|
||||
|
||||
```css
|
||||
/* 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;
|
||||
}
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run test to verify it passes**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py::test_dialog_focus_highlight_uses_accent -v`
|
||||
Expected: PASS
|
||||
|
||||
- [ ] **Step 5: Sanity-check the dialog still mounts**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py -k new_session -v`
|
||||
Expected: PASS (the CSS parses and the existing New Session tests are unaffected).
|
||||
|
||||
- [ ] **Step 6: Commit**
|
||||
|
||||
```bash
|
||||
git add src/hqt/tui/styles.tcss tests/test_tui.py
|
||||
git commit -m "style: unify New Session dialog focus highlight to accent
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 3: Soft context-aware j/k field navigation
|
||||
|
||||
**Files:**
|
||||
- Modify: `src/hqt/tui/screens/new_session.py` (add `BINDINGS` and the `Binding` import)
|
||||
- Test: `tests/test_tui.py`
|
||||
|
||||
Context: j/k move focus through the dialog's focus chain. Because `Input`
|
||||
consumes printable keys before screen bindings fire, j/k type literally while
|
||||
editing text and only move focus when the focused control is a toggle/select/
|
||||
button — the "soft" model. `focus_next`/`focus_previous` are built-in Textual
|
||||
screen actions; Tab/Shift+Tab/arrows are unaffected.
|
||||
|
||||
- [ ] **Step 1: Write the failing tests**
|
||||
|
||||
Append to `tests/test_tui.py`:
|
||||
|
||||
```python
|
||||
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
|
||||
```
|
||||
|
||||
- [ ] **Step 2: Run tests to verify they fail**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py -k "jk or jk_focus" -v`
|
||||
Expected: FAIL — `NewSessionScreen` has no `BINDINGS` attribute defining j/k (and pressing j on the switch would not move focus).
|
||||
|
||||
- [ ] **Step 3: Add the bindings**
|
||||
|
||||
In `src/hqt/tui/screens/new_session.py`, add the `Binding` import. Change:
|
||||
|
||||
```python
|
||||
from textual.app import ComposeResult
|
||||
```
|
||||
|
||||
to:
|
||||
|
||||
```python
|
||||
from textual.app import ComposeResult
|
||||
from textual.binding import Binding
|
||||
```
|
||||
|
||||
Then add a `BINDINGS` class attribute as the first line inside the
|
||||
`NewSessionScreen` class body (immediately after the `class` line, before
|
||||
`__init__`):
|
||||
|
||||
```python
|
||||
# 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),
|
||||
]
|
||||
```
|
||||
|
||||
- [ ] **Step 4: Run tests to verify they pass**
|
||||
|
||||
Run: `uv run --active pytest tests/test_tui.py -k "jk or jk_focus" -v`
|
||||
Expected: PASS (all three tests)
|
||||
|
||||
- [ ] **Step 5: Commit**
|
||||
|
||||
```bash
|
||||
git add src/hqt/tui/screens/new_session.py tests/test_tui.py
|
||||
git commit -m "feat: soft j/k field navigation in New Session dialog
|
||||
|
||||
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>"
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Task 4: Full-suite verification
|
||||
|
||||
**Files:** none (verification only)
|
||||
|
||||
- [ ] **Step 1: Run the whole test suite**
|
||||
|
||||
Run: `uv run --active pytest -q`
|
||||
Expected: PASS — no regressions; the new tests included.
|
||||
|
||||
- [ ] **Step 2: Lint**
|
||||
|
||||
Run: `uv run --active ruff check`
|
||||
Expected: `All checks passed!`
|
||||
|
||||
- [ ] **Step 3 (optional manual smoke): launch the TUI and open New Session**
|
||||
|
||||
Run the app, press `n`, and confirm: the fs/net rows appear only after toggling
|
||||
Sandboxed on; focused controls show a Peach (not Lavender) highlight; j/k move
|
||||
between toggles but type literally in the nickname/model/branch fields.
|
||||
Reference in New Issue
Block a user