From c93dc61ef63c9e4936f73f6aa47f2de9c868a096 Mon Sep 17 00:00:00 2001 From: proxyctl coordinator Date: Fri, 17 Jul 2026 23:33:56 +0800 Subject: [PATCH] pi-agent: Implement Help navigation consistency --- .../issues/10-help-dialog-and-navigation.md | 24 +- README.md | 6 +- internal/tui/active_profile_picker_test.go | 2 +- internal/tui/help_dialog.go | 215 +++++++++++++++++ internal/tui/help_dialog_test.go | 219 ++++++++++++++++++ internal/tui/model.go | 38 +-- internal/tui/operation_coordination_test.go | 8 +- internal/tui/pages.go | 67 +++--- internal/tui/presentation.go | 15 +- internal/tui/target_test.go | 14 +- 10 files changed, 544 insertions(+), 64 deletions(-) create mode 100644 internal/tui/help_dialog.go create mode 100644 internal/tui/help_dialog_test.go diff --git a/.scratch/tui-redesign/issues/10-help-dialog-and-navigation.md b/.scratch/tui-redesign/issues/10-help-dialog-and-navigation.md index 79e1b62..46adf5c 100644 --- a/.scratch/tui-redesign/issues/10-help-dialog-and-navigation.md +++ b/.scratch/tui-redesign/issues/10-help-dialog-and-navigation.md @@ -4,16 +4,16 @@ **Blocked by:** 04 — Unify operation coordination and status feedback; 07 — Build the dedicated Profile management page. -**Status:** ready-for-agent +**Status:** done -- [ ] Help opens as a scrollable contextual dialog over the current page rather than an unbounded static document. -- [ ] Help describes actions valid for the current page, focus, Target state, and operation state. -- [ ] Escape closes the top dialog or returns one page level. -- [ ] `q` returns from child pages and exits only from the dashboard. -- [ ] Ctrl+C requests exit from every page and follows deferred-exit safety during mutations. -- [ ] Closing a dialog restores the prior page, focus, selection, and viewport when still valid. -- [ ] Dialog stacking and confirmation precedence are deterministic; an input cannot accidentally act on content behind a dialog. -- [ ] Dialogs resize within the agreed terminal bounds and preserve meaningful focus. -- [ ] Help can be opened and scrolled during a mutation while conflicting actions remain disabled. -- [ ] Footer hints agree with actual accepted keys, including cancel and back behavior. -- [ ] Complete-Model tests cover Help from every page, long Help, nested dialog precedence, resize, focus restoration, `q`, Escape, Ctrl+C, and in-flight operation behavior. +- [x] Help opens as a scrollable contextual dialog over the current page rather than an unbounded static document. +- [x] Help describes actions valid for the current page, focus, Target state, and operation state. +- [x] Escape closes the top dialog or returns one page level. +- [x] `q` returns from child pages and exits only from the dashboard. +- [x] Ctrl+C requests exit from every page and follows deferred-exit safety during mutations. +- [x] Closing a dialog restores the prior page, focus, selection, and viewport when still valid. +- [x] Dialog stacking and confirmation precedence are deterministic; an input cannot accidentally act on content behind a dialog. +- [x] Dialogs resize within the agreed terminal bounds and preserve meaningful focus. +- [x] Help can be opened and scrolled during a mutation while conflicting actions remain disabled. +- [x] Footer hints agree with actual accepted keys, including cancel and back behavior. +- [x] Complete-Model tests cover Help from every page, long Help, nested dialog precedence, resize, focus restoration, `q`, Escape, Ctrl+C, and in-flight operation behavior. diff --git a/README.md b/README.md index e429b45..a1d0da6 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ The v1 binaries target Linux on amd64 and arm64. General TUI behavior and the Sh - **Docker containers/builds:** the standard per-user Docker client `config.json` model. - **GNOME:** sessions exposing the expected `org.gnome.system.proxy` settings schemas. -Unavailable Targets are hidden from the main panel; `?` shows every built-in Target and its detection evidence. +Unavailable Targets are hidden from the main panel; `?` opens contextual, scrollable Help over the current page and includes every built-in Target and its detection evidence. ## Install @@ -50,7 +50,9 @@ proxyctl Press `m` for Proxy Profile management, then `n` to create a Profile, enter a name and proxy URL, and press Ctrl+S to save. The management page supports reviewing obscured connection details, editing, and safe deletion; Active Profile switching remains a separate dashboard picker workflow rather than a management action. Single mode applies one URL to HTTP, HTTPS, and ALL/SOCKS. Ctrl+T selects advanced mode for separate values. A bare `host:port` is accepted and normalized to `http://`. With the cursor in the Single proxy URL field, Ctrl+D runs the Protocol Probe, which detects whether the endpoint speaks HTTP or SOCKS5 and fills in the scheme. When one endpoint supports both protocols, the probe switches the form to advanced mode and fills HTTP/HTTPS with `http://` and ALL/SOCKS with `socks5://`. Bypass accepts hostnames, domain suffixes, IPv4, IPv6, and CIDR entries. -The dashboard shows the Active Profile. Press `p` to open the searchable Active Profile picker, type to filter, use arrows to select, and press Enter to activate; Escape cancels without changing the Active Profile. Use arrows or `j`/`k` to select an Available Target and Space to run its contextual action: Enable, Disable, Apply Profile, or Resolve. Drift Resolution explicitly offers Reapply Active Profile and Restore Original; restoration refuses safely if proxyctl no longer has sufficient ownership evidence. Press `r` to refresh detection, `u` to show unavailable Targets, `?` for Help, and `q` to leave. Applying or reapplying requires an Active Profile, while Restore Original remains available without one. +The dashboard shows the Active Profile. Press `p` to open the searchable Active Profile picker, type to filter, use arrows to select, and press Enter to activate; Escape cancels without changing the Active Profile. Use arrows or `j`/`k` to select an Available Target and Space to run its contextual action: Enable, Disable, Apply Profile, or Resolve. Drift Resolution explicitly offers Reapply Active Profile and Restore Original; restoration refuses safely if proxyctl no longer has sufficient ownership evidence. Press `r` to refresh detection, `u` to show unavailable Targets, and `?` on any page or dialog for contextual Help. Help scrolls with arrows, `j`/`k`, Page Up/Page Down, Home, and End; `q` or Escape closes it without changing the page, focus, selection, or viewport beneath it. Applying or reapplying requires an Active Profile, while Restore Original remains available without one. + +Escape closes the top dialog or returns one page level. `q` returns from child pages and exits only from the unobscured dashboard (in the searchable Profile picker it remains search input). Ctrl+C requests exit everywhere. If a system mutation is running, either dashboard `q` or Ctrl+C visibly defers exit until the mutation completes safely; Target browsing and Help remain available while conflicting changes are disabled. Profiles are human-readable under the user's XDG configuration directory (normally `~/.config/proxyctl/profiles.json`) and are stored owner-only because proxy URLs may contain credentials. diff --git a/internal/tui/active_profile_picker_test.go b/internal/tui/active_profile_picker_test.go index 89f5a84..e809d40 100644 --- a/internal/tui/active_profile_picker_test.go +++ b/internal/tui/active_profile_picker_test.go @@ -144,7 +144,7 @@ func TestCompleteModelPickerActivationUsesProgressAndSafeDockerConfirmation(t *t close(confirmation) model, _ = applyOperationCommand(t, model, command) view := model.View() - if !strings.Contains(view, "Confirm Active Profile change") || !strings.Contains(view, "Enter/n/Esc: Cancel") { + if !strings.Contains(view, "Confirm Active Profile change") || !strings.Contains(view, "n/q/Esc: cancel") || !strings.Contains(view, "> Cancel") { t.Fatalf("activation did not use safe-default confirmation:\n%s", view) } diff --git a/internal/tui/help_dialog.go b/internal/tui/help_dialog.go new file mode 100644 index 0000000..7c8cd3a --- /dev/null +++ b/internal/tui/help_dialog.go @@ -0,0 +1,215 @@ +package tui + +import ( + "fmt" + "strings" + + "github.com/charmbracelet/lipgloss" + + "proxyctl/internal/app" +) + +// helpDialogState is an overlay, not a page. Keeping only its viewport state +// here means opening Help cannot replace the page, selection, focus, or the +// viewport beneath it. +type helpDialogState struct { + open bool + offset int + confirmationAbove bool +} + +func (h *helpDialogState) show() { + h.open = true + h.offset = 0 + // An explicitly opened Help dialog is newer than any visible confirmation. + h.confirmationAbove = false +} + +func (h *helpDialogState) close() { *h = helpDialogState{} } + +func (h helpDialogState) top(confirmation *app.Confirmation) bool { + return h.open && (confirmation == nil || !h.confirmationAbove) +} + +func (h *helpDialogState) update(key string) { + switch key { + case "up", "k": + h.offset = max(0, h.offset-1) + case "down", "j": + h.offset++ + case "pgup": + h.offset = max(0, h.offset-5) + case "pgdown": + h.offset += 5 + case "home": + h.offset = 0 + case "end": + h.offset = 1 << 20 // the viewport clamps this to the final content line + case "q", "esc": + h.close() + } +} + +func (m Model) dashboardQuitAvailable() bool { + return m.page == mainPage && !m.help.open && !m.picker.open && + m.driftResolutionTargetID == "" && m.operation.confirmation == nil +} + +func (p terminalPresentation) helpDialogView(model presentationState) terminalPage { + content := lipgloss.NewStyle().Width(max(1, model.width)).Render(p.contextualHelp(model)) + return terminalPage{ + primary: content, + footer: "↑/k ↓/j PgUp/PgDn Home/End: scroll\nq/Esc: close Help Ctrl+C: exit", + focusLine: -1, + primaryOffset: model.help.offset, + } +} + +func (p terminalPresentation) contextualHelp(model presentationState) string { + var body strings.Builder + body.WriteString(p.theme.section.Render("Contextual Help")) + body.WriteString("\nOnly the actions listed for this context are accepted while the dialog beneath Help remains unchanged.\n\n") + + switch { + case model.confirmation != nil: + body.WriteString(p.theme.section.Render("Safety confirmation")) + fmt.Fprintf(&body, "\n%s\n", model.confirmation.Title) + if model.confirmation.Summary != "" { + fmt.Fprintf(&body, "%s\n", model.confirmation.Summary) + } + body.WriteString("Left/Right or Tab changes the focused choice. Enter chooses it. y confirms. n, q, or Escape cancels. ") + if model.confirmationDialog != nil { + body.WriteString(confirmationFocusDescription(*model.confirmationDialog)) + } + body.WriteByte('\n') + case model.profileDelete != nil: + body.WriteString(p.theme.section.Render("Delete confirmation")) + fmt.Fprintf(&body, "\nDelete Proxy Profile %q. Deletion cannot be undone.\n", model.profileDelete.name) + body.WriteString("Left/Right or Tab changes the focused choice. Enter chooses it. y deletes. n, q, or Escape cancels. ") + body.WriteString(confirmationFocusDescription(model.profileDelete.dialog)) + body.WriteByte('\n') + case model.picker.open: + body.WriteString(p.theme.section.Render("Active Profile picker")) + body.WriteString("\nType to filter Proxy Profiles; q is ordinary search input here. Up/Down and PgUp/PgDn select. Enter activates the selection. Escape cancels.\n") + case model.driftResolutionTargetID != "": + body.WriteString(p.theme.section.Render("Drift Resolution dialog")) + body.WriteString("\nr reapplies the Active Profile. o restores original configuration when ownership evidence permits. Enter, q, or Escape cancels.\n") + if selected, ok := newTargetCatalog(model.snapshot, true, model.driftResolutionTargetID).selected(model.driftResolutionTargetID); ok { + for _, kind := range []app.TargetActionKind{app.ReapplyActiveProfileAction, app.RestoreOriginalAction} { + action, found := driftResolutionAction(selected, kind) + if found && action.Available { + fmt.Fprintf(&body, "%s: available.\n", targetActionLabel(kind)) + } else { + reason := action.DisabledReason + if reason == "" { + reason = "unavailable" + } + fmt.Fprintf(&body, "%s: disabled — %s.\n", targetActionLabel(kind), reason) + } + } + } + case model.page == profilesPage: + body.WriteString(p.theme.section.Render("Profile management")) + if selected, ok := profileAt(model.snapshot, model.selected); ok { + fmt.Fprintf(&body, "\nSelected Profile: %s.\n", selected.Name) + body.WriteString("Up/Down or j/k selects a Profile. PgUp/PgDn scrolls its details. n creates, e or Enter edits, and d opens the delete confirmation. q or Escape returns to the dashboard.\n") + if model.operationActive || model.exitDeferred { + body.WriteString("Profile modifications are disabled until the current mutation completes.\n") + } else { + body.WriteString("Create, Edit selected Profile, and Delete selected Profile are available.\n") + } + } else { + body.WriteString("\nNo Proxy Profiles are saved. n creates one. q or Escape returns to the dashboard.\n") + } + case model.page == formPage: + body.WriteString(p.theme.section.Render("Profile form")) + labels := model.form.labels() + if model.form.field >= 0 && model.form.field < len(labels) { + fmt.Fprintf(&body, "\nFocus: %s field.\n", labels[model.form.field]) + } + body.WriteString("Tab or Down moves to the next field; Shift+Tab or Up moves to the previous field. Ctrl+T changes mode. Ctrl+S saves. q or Escape cancels and returns to Profile management.\n") + if model.form.profile.Mode == "single" && model.form.field == 1 { + body.WriteString("Ctrl+D explicitly runs Protocol Probe for the Single proxy URL field.\n") + } + default: + body.WriteString(p.theme.section.Render("Dashboard")) + focus := "Targets focus" + if model.dashboardFocus == targetDetailsPane { + focus = "Target details focus" + } + fmt.Fprintf(&body, "\nContext: %s.\n", focus) + catalog := newTargetCatalog(model.snapshot, model.showUnavailable, model.selectedTargetID) + if selected, ok := catalog.selected(model.selectedTargetID); ok { + fmt.Fprintf(&body, "Selected Target: %s. State: %s.\n", selected.Description.Name, targetStateLabel(selected.State)) + availability := targetMutationAvailability(selected, true, model.operationActive || model.exitDeferred) + label := targetActionLabel(selected.PrimaryAction.Kind) + if availability.available { + fmt.Fprintf(&body, "Primary action: %s — available with Space.\n", label) + } else { + fmt.Fprintf(&body, "Primary action: %s — disabled: %s.\n", label, availability.reason) + } + } + if model.dashboardFocus == targetDetailsPane { + body.WriteString("Up/Down, j/k, and PgUp/PgDn scroll Target details. Tab focuses Targets.\n") + } else { + body.WriteString("Up/Down or j/k selects a Target. Tab focuses Target details.\n") + } + if model.operationActive || model.exitDeferred { + body.WriteString("u still shows or hides unavailable Targets. p, m, r, and Space modifications are disabled until the current mutation completes.\n") + } else { + body.WriteString("u shows or hides unavailable Targets. p opens the Active Profile picker. m opens Profile management. r refreshes Targets.\n") + } + } + + body.WriteString("\n") + body.WriteString(p.theme.section.Render("Operation state")) + switch { + case model.exitDeferred: + body.WriteString("\nExit is pending. Read-only browsing and Help remain available; all new work is disabled until the mutation completes safely.\n") + case model.operationActive: + body.WriteString("\nMutation in progress. Target selection, details scrolling, and Help remain available.\nConflicting mutations are disabled until completion. Ctrl+C requests a deferred safe exit.\n") + default: + body.WriteString("\nNo mutation is in progress. Ctrl+C exits immediately.\n") + } + if model.currentError != nil && model.page == mainPage && model.confirmation == nil && !model.picker.open && model.driftResolutionTargetID == "" { + body.WriteString("e expands or collapses the current error; x acknowledges it.\n") + } + + if model.page == mainPage && model.profileDelete == nil { + body.WriteString("\n") + body.WriteString(p.theme.section.Render("Built-in Target reference")) + body.WriteString("\n") + for _, item := range model.snapshot.Targets { + fmt.Fprintf(&body, "%s\n%s\n%s\n", item.Description.Name, item.Description.Summary, item.Description.Details) + if item.Available { + fmt.Fprintf(&body, "Detected: %s\n\n", item.AvailabilityEvidence) + } else { + fmt.Fprintf(&body, "Not detected: %s\n\n", item.AvailabilityEvidence) + } + } + } + + body.WriteString("\n") + body.WriteString(p.theme.section.Render("Dialog and page navigation")) + body.WriteString("\nEscape closes the top dialog. With no dialog open, Escape returns one page level. q closes Help and confirmations, returns from child pages, and exits only from the unobscured dashboard. Picker search accepts q as text.\n") + body.WriteString("Inputs are delivered only to the top dialog; they cannot activate content behind it. A newly requested safety confirmation appears above older Help.\n") + + body.WriteString("\n") + body.WriteString(p.theme.section.Render("Global exit safety")) + body.WriteString("\nCtrl+C requests exit from every page and dialog. During a system mutation, proxyctl waits for safe completion and rejects new work before exiting.\n") + return body.String() +} + +func confirmationFocusDescription(dialog confirmationDialog) string { + label := dialog.request.CancelLabel + if label == "" { + label = "Cancel" + } + if dialog.confirmFocused { + label = dialog.request.ConfirmLabel + if label == "" { + label = "Confirm" + } + } + return label + " is focused." +} diff --git a/internal/tui/help_dialog_test.go b/internal/tui/help_dialog_test.go new file mode 100644 index 0000000..7e2ef5e --- /dev/null +++ b/internal/tui/help_dialog_test.go @@ -0,0 +1,219 @@ +package tui + +import ( + "strings" + "testing" + + tea "github.com/charmbracelet/bubbletea" + + "proxyctl/internal/app" + "proxyctl/internal/profile" +) + +func TestCompleteModelContextualHelpOpensOverEveryPageAndRestoresPageState(t *testing.T) { + module := &recordingModule{snapshot: app.Snapshot{ + ActiveProfile: "office", + Profiles: []profile.Profile{ + {Name: "office", Mode: profile.SingleMode, SingleURL: "http://office.example:8080"}, + {Name: "remote", Mode: profile.SingleMode, SingleURL: "http://remote.example:8080"}, + }, + Targets: operationSnapshot().Targets, + }} + + t.Run("dashboard focus and Target selection", func(t *testing.T) { + model := resizedModel(t, module, themeModeNoColor, 120, 36) + model = updateKey(model, "down") + model = updateKey(model, "tab") + model = updateKey(model, "down") + model = updateKey(model, "?") + view := model.View() + assertUsableFrame(t, view, 120, 36, "Operations Console", "Contextual Help", "Dashboard", "Target details focus", "APT", "Disable") + + model = updateKey(model, " ") + if len(module.intents) != 0 { + t.Fatalf("input in Help acted on the dashboard: %#v", module.intents) + } + model = updateKey(model, "esc") + if model.page != mainPage || model.selectedTargetID != "apt" || model.dashboardFocus != targetDetailsPane || model.detailsOffset != 1 { + t.Fatalf("closing Help did not restore dashboard state: page=%d target=%q focus=%d offset=%d", model.page, model.selectedTargetID, model.dashboardFocus, model.detailsOffset) + } + }) + + t.Run("Profile management selection and viewport", func(t *testing.T) { + model := resizedModel(t, module, themeModeNoColor, 80, 24) + model = updateKey(model, "m") + model = updateKey(model, "down") + model = updateKey(model, "pgdown") + model = updateKey(model, "?") + assertUsableFrame(t, model.View(), 80, 24, "Proxy Profile management", "Contextual Help", "Profile management", "remote", "Edit selected Profile") + model = updateKey(model, "q") + if model.page != profilesPage || model.selectedProfileName != "remote" || model.profileDetailsOffset != 5 { + t.Fatalf("closing Help did not restore management state: page=%d Profile=%q offset=%d", model.page, model.selectedProfileName, model.profileDetailsOffset) + } + }) + + t.Run("Profile form field focus", func(t *testing.T) { + model := resizedModel(t, module, themeModeNoColor, 80, 24) + model = updateKey(model, "m") + model = updateKey(model, "e") + model = updateKey(model, "tab") + model = updateKey(model, "?") + assertUsableFrame(t, model.View(), 80, 24, "Edit Proxy Profile", "Contextual Help", "Profile form", "Single proxy URL field") + model = updateKey(model, "esc") + if model.page != formPage || model.form.field != 1 || model.form.profile.Name != "office" { + t.Fatalf("closing Help did not restore form state: page=%d field=%d Profile=%q", model.page, model.form.field, model.form.profile.Name) + } + }) +} + +func TestCompleteModelLongHelpScrollsAndSurvivesResize(t *testing.T) { + model := resizedModel(t, responsiveLayoutModule(), themeModeNoColor, 60, 16) + model = updateKey(model, "?") + initial := model.View() + assertUsableFrame(t, initial, 60, 16, "Contextual Help", "Dashboard") + if strings.Contains(initial, "Global exit safety") { + t.Fatalf("long Help unexpectedly exposed its final section before scrolling:\n%s", initial) + } + + for range 6 { + model = updateKey(model, "pgdown") + } + scrolled := model.View() + assertUsableFrame(t, scrolled, 60, 16, "Global exit safety", "Ctrl+C") + if scrolled == initial { + t.Fatal("PgDn did not scroll Help") + } + + model = resize(model, 120, 36) + model = resize(model, 60, 16) + assertUsableFrame(t, model.View(), 60, 16, "Global exit safety") + model = updateKey(model, "home") + assertUsableFrame(t, model.View(), 60, 16, "Dashboard") +} + +func TestCompleteModelHelpStacksOverConfirmationAndBlocksUnderlyingInput(t *testing.T) { + module := &profileManagementModule{snapshot: app.Snapshot{Profiles: []profile.Profile{{Name: "office"}, {Name: "remote"}}}} + model := resizedModel(t, module, themeModeNoColor, 80, 24) + model = updateKey(model, "m") + model = updateKey(model, "d") + model = updateKey(model, "?") + assertUsableFrame(t, model.View(), 80, 24, "Contextual Help", "Delete confirmation", "Deletion cannot be undone") + + model = updateKey(model, "y") + if len(module.intents) != 0 || !strings.Contains(model.View(), "Contextual Help") { + t.Fatalf("confirmation key leaked through Help: intents=%#v\n%s", module.intents, model.View()) + } + model = updateKey(model, "esc") + assertUsableFrame(t, model.View(), 80, 24, "Delete Proxy Profile “office”?", "> Cancel") + model = updateKey(model, "esc") + if len(module.intents) != 0 || strings.Contains(model.View(), "Delete Proxy Profile") { + t.Fatalf("Escape did not close dialogs from the top without mutation: intents=%#v\n%s", module.intents, model.View()) + } +} + +func TestCompleteModelDialogResizeAndHelpPreserveMeaningfulFocus(t *testing.T) { + module := &profileManagementModule{snapshot: app.Snapshot{Profiles: []profile.Profile{{Name: "office"}, {Name: "remote"}}}} + model := resizedModel(t, module, themeModeNoColor, 80, 24) + model = updateKey(model, "m") + model = updateKey(model, "d") + model = updateKey(model, "right") + model = resize(model, 60, 16) + model = updateKey(model, "?") + assertUsableFrame(t, model.View(), 60, 16, "Delete confirmation", "Delete is focused") + model = resize(model, 120, 36) + model = updateKey(model, "esc") + assertUsableFrame(t, model.View(), 120, 36, "> Delete") + + updated, command := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) + model = updated.(Model) + if command != nil || len(module.intents) != 1 || module.intents[0].Kind != app.DeleteProfile { + t.Fatalf("resize and Help changed confirmation focus: intents=%#v", module.intents) + } +} + +func TestCompleteModelNewConfirmationTakesPrecedenceOverOpenHelp(t *testing.T) { + updates := make(chan app.OperationUpdate, 1) + module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{updates}} + model := resizedModel(t, module, themeModeNoColor, 120, 36) + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + model = updateKey(model, "?") + + updates <- app.ConfirmationOperationUpdate(module.snapshot, app.Confirmation{Title: "Confirm Docker restart", Summary: "Running work may be disrupted."}) + close(updates) + model, _ = applyOperationCommand(t, model, command) + view := model.View() + assertUsableFrame(t, view, 120, 36, "Confirm Docker restart", "> Cancel") + if strings.Contains(view, "Contextual Help") { + t.Fatalf("new safety confirmation did not take precedence over older Help:\n%s", view) + } + + model = updateKey(model, "esc") + assertUsableFrame(t, model.View(), 120, 36, "Contextual Help", "No mutation is in progress") + model = updateKey(model, "esc") + if strings.Contains(model.View(), "Contextual Help") { + t.Fatal("second Escape did not close the revealed Help dialog") + } +} + +func TestCompleteModelGlobalBackQuitAndInterruptSemantics(t *testing.T) { + module := &recordingModule{snapshot: app.Snapshot{Profiles: []profile.Profile{{Name: "office"}}}} + model := resizedModel(t, module, themeModeNoColor, 80, 24) + model = updateKey(model, "m") + model = updateKey(model, "e") + model = updateKey(model, "q") + if model.page != profilesPage { + t.Fatalf("q from form page = %d, want profilesPage", model.page) + } + model = updateKey(model, "q") + if model.page != mainPage { + t.Fatalf("q from management page = %d, want mainPage", model.page) + } + updated, quit := model.Update(keyMessage("q")) + model = updated.(Model) + if quit == nil { + t.Fatal("q from unobscured dashboard did not request exit") + } + if _, ok := quit().(tea.QuitMsg); !ok { + t.Fatal("q from dashboard did not emit tea.Quit") + } + + for _, setup := range []func(Model) Model{ + func(model Model) Model { return model }, + func(model Model) Model { return updateKey(model, "m") }, + func(model Model) Model { return updateKey(updateKey(model, "m"), "e") }, + func(model Model) Model { return updateKey(model, "?") }, + } { + candidate := setup(resizedModel(t, module, themeModeNoColor, 80, 24)) + _, command := candidate.Update(tea.KeyMsg{Type: tea.KeyCtrlC}) + if command == nil { + t.Fatalf("Ctrl+C from page %d did not request exit", candidate.page) + } + if _, ok := command().(tea.QuitMsg); !ok { + t.Fatalf("Ctrl+C from page %d did not emit tea.Quit", candidate.page) + } + } +} + +func TestCompleteModelHelpDuringMutationExplainsAndPreservesDisabledActions(t *testing.T) { + updates := make(chan app.OperationUpdate, 1) + module := &interactiveScriptModule{snapshot: operationSnapshot(), streams: []chan app.OperationUpdate{updates}} + model := resizedModel(t, module, themeModeNoColor, 60, 16) + updated, command := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + model = updateKey(model, "?") + for range 3 { + model = updateKey(model, "pgdown") + } + view := model.View() + assertUsableFrame(t, view, 60, 16, "Mutation in progress", "Conflicting mutations are disabled") + + model = updateKey(model, "esc") + updated, blocked := model.Update(tea.KeyMsg{Type: tea.KeySpace}) + model = updated.(Model) + if blocked != nil || len(module.intents) != 1 || !strings.Contains(model.View(), "Wait for the current mutation to finish") { + t.Fatalf("Help browsing changed mutation serialization: intents=%#v\n%s", module.intents, model.View()) + } + close(updates) + _ = command +} diff --git a/internal/tui/model.go b/internal/tui/model.go index b3fce60..8de7c3d 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -19,7 +19,6 @@ const ( mainPage page = iota profilesPage formPage - helpPage ) type formState struct { @@ -49,6 +48,7 @@ type Model struct { driftResolutionTargetID string picker profilePickerState form formState + help helpDialogState status string operation operationCoordinator detect func(context.Context, string) (probe.Capabilities, error) @@ -139,12 +139,19 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { return m, nil } if key, ok := message.(tea.KeyMsg); ok { - // q exits the unobscured dashboard. While the searchable picker is open, - // it is ordinary filter input; Escape remains deterministic cancellation. - exitRequest := key.String() == "ctrl+c" || (m.page == mainPage && !m.picker.open && key.String() == "q") - if exitRequest { + // Ctrl+C is global. q exits only the dashboard when no dialog obscures it; + // otherwise the top dialog or child page owns q. + if key.String() == "ctrl+c" || (key.String() == "q" && m.dashboardQuitAvailable()) { return m, m.operation.requestQuit() } + // Help is available from every page and dialog. Opening it explicitly + // places it above the current dialog without changing state underneath. + if key.String() == "?" { + if !m.help.top(m.operation.confirmation) { + m.help.show() + } + return m, nil + } } if progress, ok := message.(operationProgress); ok { if !m.operation.active || progress.updates != m.operation.updates { @@ -166,6 +173,11 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { } if m.operation.confirmation != nil { m.page = mainPage + if m.help.open { + // A safety confirmation requested after Help was opened is the new + // top dialog. Cancelling it reveals the unchanged Help beneath. + m.help.confirmationAbove = true + } } if progress.update.IsTerminal() { return m, m.operation.deferredQuit() @@ -220,6 +232,10 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { if !ok { return m, nil } + if m.help.top(m.operation.confirmation) { + m.help.update(key.String()) + return m, nil + } switch m.page { case mainPage: return m.updateMain(key) @@ -227,11 +243,6 @@ func (m Model) Update(message tea.Msg) (tea.Model, tea.Cmd) { return m.updateProfiles(key) case formPage: return m.updateForm(key) - case helpPage: - if key.String() == "q" || key.String() == "esc" { - m.page = mainPage - } - return m, nil default: return m, nil } @@ -314,7 +325,7 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { } if m.driftResolutionTargetID != "" { switch key.String() { - case "enter", "esc", "n": + case "enter", "esc", "n", "q": m.driftResolutionTargetID = "" return m, nil case "r", "o": @@ -381,8 +392,6 @@ func (m Model) updateMain(key tea.KeyMsg) (tea.Model, tea.Cmd) { m.operation.status = app.OperationStatus{} } } - case "?": - m.page = helpPage case "r": if m.operation.active || m.operation.quitRequested { m.operation.status = app.OperationStatus{Kind: app.StatusBlocked, Summary: "Wait for the current mutation to finish before refreshing Targets"} @@ -546,7 +555,7 @@ func newProfileDeleteConfirmation(selected profile.Profile, final, active bool) func (m Model) updateForm(key tea.KeyMsg) (tea.Model, tea.Cmd) { switch key.String() { - case "esc": + case "q", "esc": m.page = profilesPage m.status = "" m.form = formState{} @@ -723,6 +732,7 @@ func (m Model) View() string { driftResolutionTargetID: m.driftResolutionTargetID, picker: m.picker, form: m.form, + help: m.help, confirmation: m.operation.confirmation, confirmationDialog: &m.operation.confirmationDialog, status: m.status, diff --git a/internal/tui/operation_coordination_test.go b/internal/tui/operation_coordination_test.go index 8f9fbe2..e30febf 100644 --- a/internal/tui/operation_coordination_test.go +++ b/internal/tui/operation_coordination_test.go @@ -70,8 +70,8 @@ func TestCompleteModelKeepsDashboardAndHelpBrowsableWhileMutationIsSerialized(t model = updateKey(model, "tab") model = updateKey(model, "down") model = updateKey(model, "?") - if model.page != helpPage || !strings.Contains(model.View(), "proxyctl Help") { - t.Fatalf("Help was not reachable while mutation ran:\n%s", model.View()) + if model.page != mainPage || !model.help.open || !strings.Contains(model.View(), "Contextual Help") { + t.Fatalf("Help was not reachable as a dashboard dialog while mutation ran:\n%s", model.View()) } model = updateKey(model, "q") if model.page != mainPage { @@ -179,7 +179,7 @@ func TestCompleteModelUsesStructuredConfirmationAndSafeCancellation(t *testing.T close(confirmation) model, _ = applyOperationCommand(t, model, command) view := model.View() - assertUsableFrame(t, view, 120, 36, "Confirm Docker restart", "Running work may be disrupted", "Enter/n/Esc: Cancel") + assertUsableFrame(t, view, 120, 36, "Confirm Docker restart", "Running work may be disrupted", "n/q/Esc: cancel", "> Cancel") updated, next := model.Update(tea.KeyMsg{Type: tea.KeyEnter}) model = updated.(Model) @@ -234,7 +234,7 @@ func TestCompleteModelVisiblyDefersCtrlCFromHelpAndStartsNoNewWork(t *testing.T) t.Fatalf("deferred exit prevented read-only Target browsing: selected=%q", model.selectedTargetID) } model = updateKey(model, "?") - if model.page != helpPage { + if model.page != mainPage || !model.help.open { t.Fatal("deferred exit prevented opening read-only Help") } model = updateKey(model, "q") diff --git a/internal/tui/pages.go b/internal/tui/pages.go index d1e349a..30f9729 100644 --- a/internal/tui/pages.go +++ b/internal/tui/pages.go @@ -23,7 +23,7 @@ func (p terminalPresentation) mainView(model presentationState, width, height in catalog := newTargetCatalog(model.snapshot, model.showUnavailable, model.selectedTargetID) footer := p.dashboardFooter(model, catalog) if model.confirmation != nil && !model.operationActive { - footer = "y: confirm Enter/n/Esc: Cancel q: exit" + footer = "←/→/Tab: choose y: confirm Enter: choose n/q/Esc: cancel ?: help" } if model.currentError != nil { footer += "\ne: expand/collapse current error x: acknowledge error" @@ -45,13 +45,16 @@ func (p terminalPresentation) mainView(model presentationState, width, height in if model.picker.open { page.dashboard = nil page.primary = model.picker.view(model.snapshot, width, max(5, height-9)) - page.footer = "↑/↓ PgUp/PgDn: select Enter: activate Esc: cancel" + page.footer = "Type to search ↑/↓ PgUp/PgDn: select Enter: activate\nEsc: cancel ?: help" + if width < 70 { + page.footer = "Type: search ↑/↓: select Enter: activate\nEsc: cancel ?: help" + } } if model.driftResolutionTargetID != "" && model.confirmation == nil { if selected, ok := newTargetCatalog(model.snapshot, true, model.driftResolutionTargetID).selected(model.driftResolutionTargetID); ok { page.dashboard = nil page.primary, page.context = p.driftResolutionView(selected) - page.footer = "r: Reapply Active Profile o: Restore Original\nEnter/Esc: Cancel" + page.footer = "r: Reapply Active Profile o: Restore Original\nEnter/q/Esc: cancel ?: help" } } if model.confirmation != nil { @@ -62,6 +65,7 @@ func (p terminalPresentation) mainView(model presentationState, width, height in page.dashboard = nil page.primary = dialog.view() page.context = "Cancellation is selected by default." + page.footer = "←/→/Tab: choose y: confirm Enter: choose\nn/q/Esc: cancel ?: help" } return page } @@ -95,7 +99,16 @@ func (p terminalPresentation) dashboardFooter(model presentationState, catalog t visibility = "u: hide unavailable" } if model.dashboardFocus == targetDetailsPane { - footer := "↑/k ↓/j PgUp/PgDn: scroll Tab: focus Targets " + visibility + "\np: switch Active Profile m: manage Proxy Profiles ?: help q: quit" + if model.width < wideWidth { + if model.operationActive { + return "↑/k ↓/j PgUp/PgDn: browse Tab: focus Targets\n?: help q/Ctrl+C: defer exit; mutations disabled" + } + if model.width < 70 { + return "↑/k ↓/j PgUp/PgDn: scroll Tab: focus Targets\np: switch Profile m: manage ?: help q: quit" + } + return "↑/k ↓/j PgUp/PgDn: scroll Tab: focus Targets\np: switch Active Profile m: manage Proxy Profiles ?: help q: quit" + } + footer := "↑/k ↓/j PgUp/PgDn: scroll Tab: focus Targets " + visibility + "\np: switch Active Profile m: manage Proxy Profiles ?: help q/Ctrl+C: quit" if model.operationActive { footer = "↑/k ↓/j PgUp/PgDn: scroll Tab: focus Targets ?: help\nMutations disabled: wait for the current mutation to finish" } @@ -110,31 +123,28 @@ func (p terminalPresentation) dashboardFooter(model presentationState, catalog t } else { actions = "Space: " + label + " disabled (" + availability.compactReason + ") " + actions } - footer := actions + "\np: switch Active Profile m: manage Proxy Profiles r: refresh ?: help q: quit" + if model.width < wideWidth { + if availability.available { + actions = "Space: " + label + " ↑/k ↓/j: select Tab: details" + } else { + actions = "Space: " + label + " disabled ↑/k ↓/j: select Tab: details" + } + + if model.operationActive { + return actions + "\n?: help q/Ctrl+C: defer exit; mutations disabled" + } + if model.width < 70 { + return actions + "\np: switch Profile m: manage ?: help q: quit" + } + return actions + "\np: switch Active Profile m: manage Proxy Profiles ?: help q: quit" + } + footer := actions + "\np: switch Active Profile m: manage Proxy Profiles r: refresh ?: help q/Ctrl+C: quit" if model.operationActive { footer = actions + "\n?: help q: defer safe exit; new mutations disabled" } return footer } -func (p terminalPresentation) helpView(model presentationState) terminalPage { - var body strings.Builder - for _, item := range model.snapshot.Targets { - fmt.Fprintf(&body, "%s\n%s\n%s\n", p.theme.section.Render(item.Description.Name), item.Description.Summary, item.Description.Details) - if item.Available { - fmt.Fprintf(&body, "Detected: %s\n\n", item.AvailabilityEvidence) - } else { - fmt.Fprintf(&body, "Not detected: %s\n\n", item.AvailabilityEvidence) - } - } - return terminalPage{ - header: p.theme.title.Render("proxyctl Help — built-in Targets"), - primary: strings.TrimRight(body.String(), "\n"), - footer: "↑/k ↓/j: navigate Space: toggle p: switch Active Profile\nm: manage Proxy Profiles r: refresh ?: help q/Escape: back", - focusLine: 0, - } -} - func (p terminalPresentation) profilesView(model presentationState) terminalPage { var list strings.Builder if len(model.snapshot.Profiles) == 0 { @@ -155,15 +165,18 @@ func (p terminalPresentation) profilesView(model presentationState) terminalPage } context := selectedProfileDetails(p, model) - footer := "n: create e: edit Enter: edit d: delete PgUp/PgDn: scroll details\n↑/k ↓/j: select q/Esc: back" + footer := "n: create e: edit Enter: edit d: delete PgUp/PgDn: scroll details\n↑/k ↓/j: select ?: help q/Esc: back" + if model.width < 80 { + footer = "↑/k ↓/j: select PgUp/PgDn: details n: create\ne/Enter: edit d: delete ?: help q/Esc: back" + } if len(model.snapshot.Profiles) == 0 { - footer = "n: create a Proxy Profile q/Esc: back" + footer = "n: create a Proxy Profile ?: help q/Esc: back" } if model.profileDelete != nil { return terminalPage{ header: p.theme.title.Render("Proxy Profile management"), primary: model.profileDelete.dialog.view(), - footer: "←/→/Tab: choose Enter: choose y: delete n/q/Esc: cancel", + footer: "←/→/Tab: choose Enter: choose y: delete\nn/q/Esc: cancel ?: help", focusLine: -1, } } @@ -226,7 +239,7 @@ func (p terminalPresentation) formView(model presentationState) terminalPage { return terminalPage{ header: p.theme.title.Render(fmt.Sprintf("%s Proxy Profile — %s mode", action, model.form.profile.Mode)), primary: fields.String(), - footer: "Tab/↑/↓: move Ctrl+T: toggle Single/Advanced\nCtrl+S: save Ctrl+D: detect protocol Esc: cancel", + footer: "Tab/↑/↓: move Ctrl+T: mode Ctrl+S: save\nCtrl+D: probe ?: help q/Esc: cancel", focusLine: model.form.field, } } diff --git a/internal/tui/presentation.go b/internal/tui/presentation.go index 340c41e..8f6cefe 100644 --- a/internal/tui/presentation.go +++ b/internal/tui/presentation.go @@ -68,6 +68,7 @@ type presentationState struct { driftResolutionTargetID string picker profilePickerState form formState + help helpDialogState confirmation *app.Confirmation confirmationDialog *confirmationDialog status string @@ -122,11 +123,21 @@ func (p terminalPresentation) Render(state presentationState) string { page = p.profilesView(state) case formPage: page = p.formView(state) - case helpPage: - page = p.helpView(state) default: page = p.mainView(state, width, height) } + if state.help.top(state.confirmation) { + help := p.helpDialogView(state) + // Retain the underlying page header so Help is visibly contextual and + // modal rather than a replacement navigation destination. + page.primary = help.primary + page.context = "" + page.footer = help.footer + page.focusLine = help.focusLine + page.primaryOffset = help.primaryOffset + page.contextOffset = 0 + page.dashboard = nil + } page.status = renderStatus(state) page.statusExpanded = state.errorExpanded && state.currentError != nil page.statusOffset = state.errorOffset diff --git a/internal/tui/target_test.go b/internal/tui/target_test.go index 54ab469..6277b01 100644 --- a/internal/tui/target_test.go +++ b/internal/tui/target_test.go @@ -114,8 +114,18 @@ func TestHelpListsEveryTargetAndMainListsOnlyAvailableTargets(t *testing.T) { } model = updateKey(model, "?") view = model.View() - if !strings.Contains(view, "Shell") || !strings.Contains(view, "APT") || !strings.Contains(view, "Not detected: Debian APT tool apt-config was not detected") || !strings.Contains(view, "owns an overlay") { - t.Fatalf("Target help:\n%s", view) + if !strings.Contains(view, "Contextual Help") || !strings.Contains(view, "Shell") { + t.Fatalf("contextual Target help:\n%s", view) + } + for range 12 { + if strings.Contains(view, "Not detected: Debian APT tool apt-config was not detected") && strings.Contains(view, "owns an overlay") { + break + } + model = updateKey(model, "pgdown") + view = model.View() + } + if !strings.Contains(view, "APT") || !strings.Contains(view, "Not detected: Debian APT tool apt-config was not detected") || !strings.Contains(view, "owns an overlay") { + t.Fatalf("scrollable Target reference:\n%s", view) } model = updateKey(model, "esc") if !strings.Contains(model.View(), "Operations Console") {