Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions internal/tui/keybinding_help.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,18 @@ func (m model) renderKeybindingHelpOverlay(width int, height int) string {
return centerRenderedBlock(block, width)
}

// keybindingHelpOverlay returns the `?` shortcut overlay as a centered block to
// COMPOSITE over the transcript — the same viewport-overlay path the model
// picker and suggestions use — or "" when it is not open. Rendering it this way
// (rather than replacing the whole View) keeps the chat visible behind it, so
// `?` behaves like every other popup instead of blanking the screen (#419).
func (m model) keybindingHelpOverlay(width int) string {
if !m.helpOverlay {
return ""
}
return m.renderKeybindingHelpOverlay(width, m.height)
}

// keybindingHelpOverlayWidth picks the overlay width: wide enough that the
// descriptions don't truncate next to the key column, capped, and never wider
// than the terminal.
Expand Down
53 changes: 53 additions & 0 deletions internal/tui/keybinding_help_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,3 +105,56 @@ func TestKeybindingGroupsAreWellFormed(t *testing.T) {
}
}
}

// #419: the `?` help overlay must render ON TOP of the chat (like the model
// picker), not REPLACE the whole screen. The old full-screen replace produced
// only the centered shortcut block on a blank canvas — no title bar, no
// composer. So with the overlay open, the surrounding chat chrome that is NOT
// covered by the centered box (the model title bar at top, the composer at
// bottom) must still be present alongside "Keyboard Shortcuts".
func TestHelpOverlayCompositesOverChatNotReplacingIt(t *testing.T) {
m := newModel(context.Background(), Options{ModelName: "gpt-4o"})
m.width = 100
m.height = 40
m.altScreen = true

base := plainRender(t, m.View()) // no overlay: baseline chrome
m.helpOverlay = true
over := plainRender(t, m.View())

if !strings.Contains(over, "Keyboard Shortcuts") {
t.Fatalf("help overlay not rendered:\n%s", over)
}
// Chrome that renders in the baseline (and sits outside the centered overlay
// box) must survive behind the overlay. The full-screen replace showed none
// of it.
for _, marker := range []string{"gpt-4o", "describe a task"} {
if !strings.Contains(base, marker) {
t.Fatalf("precondition: baseline chat should contain %q:\n%s", marker, base)
}
if !strings.Contains(over, marker) {
t.Fatalf("#419: help replaced the chat instead of overlaying it; %q is gone:\n%s", marker, over)
}
}
}

// A populated transcript row also survives behind the overlay (peeking out to
// the left of the centered box), proving the chat body — not just the chrome —
// is composited under the overlay rather than discarded.
func TestHelpOverlayKeepsTranscriptBodyBehindIt(t *testing.T) {
m := newModel(context.Background(), Options{ModelName: "gpt-4o"})
m.width = 120
m.height = 40
m.altScreen = true
m.transcript = appendTranscriptRow(m.transcript, transcriptRow{kind: rowUser, text: "hello there this is a chat line"})
m.helpOverlay = true

view := plainRender(t, m.View())
if !strings.Contains(view, "Keyboard Shortcuts") {
t.Fatalf("help overlay not rendered:\n%s", view)
}
// The start of the transcript line peeks to the left of the centered box.
if !strings.Contains(view, "hello") {
t.Fatalf("#419: transcript body was replaced by the help overlay:\n%s", view)
}
}
Comment on lines +108 to +160

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find what conditions gate sidebarActive()/two-column rendering, to help craft a targeted test.
rg -n 'func \(m model\) sidebarActive' internal/tui -A20

Repository: Gitlawb/zero

Length of output: 1692


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the view dispatch and the two-column path.
ast-grep outline internal/tui/model.go --view expanded | sed -n '1,220p'
printf '\n--- sidebar ---\n'
cat -n internal/tui/sidebar.go | sed -n '1,220p'
printf '\n--- tests mentioning help overlay or two-column ---\n'
rg -n 'helpOverlay|Keyboard Shortcuts|twoColumnTranscriptView|sidebarActive|sidebarAvailable' internal/tui -g '*_test.go' -A4 -B4

Repository: Gitlawb/zero

Length of output: 37675


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the main view dispatch and the two-column transcript implementation.
rg -n 'func \(m model\) View|func \(m model\) transcriptView|func \(m model\) twoColumnTranscriptView|helpOverlay' internal/tui/model.go internal/tui/sidebar.go -A80 -B20

Repository: Gitlawb/zero

Length of output: 38802


Add a regression test for the two-column help overlay path.
The new cases only cover the single-column transcript render; when sidebarActive() is true, transcriptView() switches to twoColumnTranscriptView(), and that branch now composites ? separately. Add one alt-screen model with an active sidebar and helpOverlay = true so this path stays covered.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/tui/keybinding_help_test.go` around lines 108 - 160, Add a
regression test for the two-column help overlay path in the existing help
overlay tests. The current coverage only exercises the single-column transcript
render, so create an alt-screen model with sidebarActive() behavior enabled and
helpOverlay set to true, then verify the overlay still appears while the
surrounding chat chrome/transcript remains visible. Use the transcriptView() and
twoColumnTranscriptView() paths as the key symbols to target this branch.

12 changes: 9 additions & 3 deletions internal/tui/model.go
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
m = m.disarmCancelConfirmation()
}
// The `?` help overlay is modal: `?`, Esc, q, or Enter close it; every
// other key is swallowed so nothing types into the hidden composer.
// other key is swallowed so nothing types into the composer behind it.
if m.helpOverlay {
if keyText(msg) == "?" || keyText(msg) == "q" || keyIs(msg, tea.KeyEsc) || keyIs(msg, tea.KeyEnter) || keyCtrl(msg, 'c') {
m.helpOverlay = false
Expand Down Expand Up @@ -2118,8 +2118,6 @@ func (m model) View() tea.View {
var content string
if m.setup.visible {
content = m.setupView(chatWidth(m.width))
} else if m.helpOverlay {
content = m.renderKeybindingHelpOverlay(chatWidth(m.width), m.height)
} else if m.transcriptDetailed {
content = m.detailedTranscriptView()
} else {
Expand Down Expand Up @@ -2194,13 +2192,16 @@ func (m model) transcriptView() string {
return body + footer
}

helpOverlay := m.keybindingHelpOverlay(width)
suggestionOverlay := m.suggestionOverlay(width)
providerOverlay := m.providerWizardOverlay(width)
mcpAddOverlay := m.mcpAddWizardOverlay(width)
mcpOverlay := m.mcpManagerOverlay(width)
pickerOverlay := m.pickerOverlay(width)
viewportOverlay := ""
switch {
case helpOverlay != "":
viewportOverlay = helpOverlay
case providerOverlay != "":
viewportOverlay = providerOverlay
case mcpAddOverlay != "":
Expand Down Expand Up @@ -2262,6 +2263,11 @@ func (m model) twoColumnTranscriptView() string {
if m.transcriptEmpty() && !m.pending {
overlayForViewport = ""
}
// The `?` help overlay composites over the chat like the pickers — set it
// after the empty-transcript clear so a user-toggled overlay always shows.
if helpOverlay := m.keybindingHelpOverlay(width); helpOverlay != "" {
overlayForViewport = helpOverlay
}

header := m.pinnedTitleBar(width)
chatBlock := viewLines(m.scrollableTranscriptItemsView(header, bodyItems, footer, width, overlayForViewport))
Expand Down
Loading