Skip to content
Merged
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
2 changes: 1 addition & 1 deletion ui/model/filebrowser.go
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ func (m *Model) handleFileBrowserKey(msg tea.KeyPressMsg) tea.Cmd {
m.fileBrowser.filtered = nil

case "ctrl+x":
m.toggleExpandPlaylist()
m.toggleExpandedView()
m.fbMaybeAdjustScroll(m.fbVisible())

case "/":
Expand Down
2 changes: 1 addition & 1 deletion ui/model/keymap.go
Original file line number Diff line number Diff line change
Expand Up @@ -333,7 +333,7 @@ func (m *Model) handleKeymapKey(msg tea.KeyPressMsg) tea.Cmd {
m.keymapMaybeAdjustScroll(m.keymapVisible())

case "ctrl+x":
m.toggleExpandPlaylist()
m.toggleExpandedView()
m.keymapMaybeAdjustScroll(m.keymapVisible())

case "pgup", "ctrl+u":
Expand Down
58 changes: 48 additions & 10 deletions ui/model/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,8 @@ func (m *Model) handleKey(msg tea.KeyPressMsg) tea.Cmd {
m.lyrics.scroll++
}
}
case "ctrl+x":
m.toggleExpandedView()
}
return nil
}
Expand Down Expand Up @@ -358,7 +360,7 @@ func (m *Model) handleKey(msg tea.KeyPressMsg) tea.Cmd {
case "R":
return m.switchToProvider("radio")
case "ctrl+x":
m.toggleExpandPlaylist()
m.toggleExpandedView()
case "ctrl+f":
m.openProviderSearch()
}
Expand Down Expand Up @@ -748,7 +750,7 @@ func (m *Model) handleKey(msg tea.KeyPressMsg) tea.Cmd {

case "ctrl+x":
if m.focus == focusPlaylist {
m.toggleExpandPlaylist()
m.toggleExpandedView()
}

case "x":
Expand Down Expand Up @@ -1023,8 +1025,8 @@ func (m *Model) updateProvSearch() {
}
}

// toggleExpandPlaylist toggles the playlist panel between default and expanded height.
func (m *Model) toggleExpandPlaylist() {
// toggleExpandedView toggles the UI between default and expanded height.
func (m *Model) toggleExpandedView() {
m.heightExpanded = !m.heightExpanded
m.applyHeightMode()
m.adjustScroll()
Expand Down Expand Up @@ -1632,24 +1634,60 @@ func (m *Model) handleThemeKey(msg tea.KeyPressMsg) tea.Cmd {
case "ctrl+c":
m.themePickerCancel()
return m.quit()

case "up", "k":
if m.themePicker.cursor > 0 {
m.themePicker.cursor--
m.themePickerApply() // live preview
} else {
} else if count > 0 {
m.themePicker.cursor = count - 1
m.themePickerApply() // live preview
}
m.themePickerApply()
m.themePickerMaybeAdjustScroll(m.themePickerVisible())

case "down", "j":
if m.themePicker.cursor < count-1 {
m.themePicker.cursor++
m.themePickerApply() // live preview
} else {
} else if count > 0 {
m.themePicker.cursor = 0
m.themePickerApply() // live preview
}
m.themePickerApply()
m.themePickerMaybeAdjustScroll(m.themePickerVisible())

case "ctrl+x":
m.toggleExpandedView()
m.themePickerMaybeAdjustScroll(m.themePickerVisible())

case "pgup", "ctrl+u":
if m.themePicker.cursor > 0 {
visible := m.themePickerVisible()
m.themePicker.cursor -= min(m.themePicker.cursor, visible)
m.themePickerApply()
m.themePickerMaybeAdjustScroll(visible)
}

case "pgdown", "ctrl+d":
if m.themePicker.cursor < count-1 {
visible := m.themePickerVisible()
m.themePicker.cursor = min(count-1, m.themePicker.cursor+visible)
m.themePickerApply()
m.themePickerMaybeAdjustScroll(visible)
}

case "home", "g":
m.themePicker.cursor = 0
m.themePickerApply()
m.themePickerMaybeAdjustScroll(m.themePickerVisible())

case "end", "G":
if count > 0 {
m.themePicker.cursor = count - 1
}
m.themePickerApply()
m.themePickerMaybeAdjustScroll(m.themePickerVisible())

case "enter":
m.themePickerSelect()

case "esc", "q", "t":
m.themePickerCancel()
}
Expand Down
10 changes: 10 additions & 0 deletions ui/model/lyrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,3 +54,13 @@ func (m *Model) lyricsHaveTimestamps() bool {
}
return false
}

// lyricsVisibleHeight returns the number of lyrics lines to show.
func (m Model) lyricsVisibleHeight() int {
limit := maxPlVisible
if m.heightExpanded {
limit = m.height
}
// Fixed overhead: header (2) + spacing/footer (2) = 4.
return max(3, min(limit, m.height-4))
}
Comment on lines +58 to +66
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | 💤 Low value

Helper looks correct; just verify the overhead constant stays in sync with the renderer.

max(3, min(limit, m.height-4)) mirrors the structure used by keymapVisible / fbVisible / themePickerVisible. The hard-coded 4 corresponds to the renderer's "title (1) + blank (1) + blank (1) + helpKey footer (1)" overhead. That's currently correct (renderLyricsOverlay in view_overlays.go opens with titleStyle… + "" and closes with "" + helpKey), but it's an implicit coupling — if anyone adds, say, a subtitle line to the header the overhead drifts silently.

Two small things to consider:

  1. Tie the constant to a named value (e.g. lyricsOverheadLines = 4) shared with the renderer, or compute it via the same lipgloss.Height(probeFrame) probe pattern the other overlays use.
  2. The receiver here is value (m Model) while lyricsArtistTitle / lyricsSyncable in the same file use pointer receivers. Minor, but (m *Model) would avoid copying the model on every render call.
🤖 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 `@ui/model/lyrics.go` around lines 58 - 66, The helper lyricsVisibleHeight
currently uses a hard-coded overhead (m.height-4) and a value receiver; change
it to use a named constant (e.g. lyricsOverheadLines = 4) or compute the
overhead via the same lipgloss.Height(probeFrame) probe pattern used by
renderLyricsOverlay to keep renderer and helper in sync, and change the receiver
from (m Model) to a pointer receiver (m *Model) to avoid copying; update
references to lyricsVisibleHeight and ensure renderLyricsOverlay and related
functions (lyricsArtistTitle, lyricsSyncable) use the shared constant or probe
logic.

53 changes: 53 additions & 0 deletions ui/model/overlays.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ package model
import (
"strings"

"charm.land/lipgloss/v2"

"cliamp/theme"
"cliamp/ui"
)

// openThemePicker re-loads themes from disk (picking up new user files)
Expand All @@ -15,6 +18,8 @@ func (m *Model) openThemePicker() {
// Position cursor on the currently active theme.
// Picker list: 0 = Default, 1..N = themes[0..N-1]
m.themePicker.cursor = m.themeIdx + 1
m.themePicker.scroll = 0
m.themePickerMaybeAdjustScroll(m.themePickerVisible())
}

// themePickerApply applies the theme under the cursor for live preview.
Expand Down Expand Up @@ -45,6 +50,54 @@ func (m *Model) themePickerCancel() {
m.themePicker.visible = false
}

func (m *Model) themePickerHelpLine() string {
return helpKey("↓↑", "Scroll ") + helpKey("Enter", "Select ") + helpKey("Esc", "Close")
}
Comment on lines +53 to +55
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Help line omits the new keys you just wired up.

handleThemeKey now supports pgup/pgdn, ctrl+u/d, home/g, end/G, and ctrl+x, but the footer only advertises ↓↑ / Enter / Esc. Discoverability for the new responsive-height + paging behavior suffers. Consider extending — at minimum mention Ctrl+X (Expand) since that's the headline of this PR.

♻️ Suggested footer hint additions
 func (m *Model) themePickerHelpLine() string {
-	return helpKey("↓↑", "Scroll ") + helpKey("Enter", "Select ") + helpKey("Esc", "Close")
+	return helpKey("↓↑", "Scroll ") + helpKey("PgUp/Dn", "Page ") +
+		helpKey("Ctrl+X", "Expand ") + helpKey("Enter", "Select ") + helpKey("Esc", "Close")
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
func (m *Model) themePickerHelpLine() string {
return helpKey("↓↑", "Scroll ") + helpKey("Enter", "Select ") + helpKey("Esc", "Close")
}
func (m *Model) themePickerHelpLine() string {
return helpKey("↓↑", "Scroll ") + helpKey("PgUp/Dn", "Page ") +
helpKey("Ctrl+X", "Expand ") + helpKey("Enter", "Select ") + helpKey("Esc", "Close")
}
🤖 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 `@ui/model/overlays.go` around lines 53 - 55, The footer help line in
themePickerHelpLine currently only shows "↓↑ / Enter / Esc" but must include the
newly supported keys from handleThemeKey (PgUp/PgDn, Ctrl+U/Ctrl+D, Home/g,
End/G) and at minimum Ctrl+X for Expand; update themePickerHelpLine to add
helpKey entries for these keys (e.g., "PgUp/PgDn", "Ctrl+U/Ctrl+D", "Home/g",
"End/G", "Ctrl+X") so the footer reflects the responsive-height and paging
controls and improves discoverability.


func (m *Model) themePickerVisible() int {
probeSections := []string{
titleStyle.Render("T H E M E S"),
"",
"x", // 1-line list placeholder
"",
dimStyle.Render(" 0/0 themes"),
"",
m.themePickerHelpLine(),
}

probeFrame := ui.FrameStyle.Render(strings.Join(probeSections, "\n"))
fixedHeight := lipgloss.Height(probeFrame) - 1

limit := maxPlVisible
if m.heightExpanded {
limit = m.height
}
return max(3, min(limit, m.height-fixedHeight))
}

func (m *Model) themePickerMaybeAdjustScroll(visible int) {
if visible <= 0 {
return
}
count := len(m.themes) + 1
if m.themePicker.cursor < 0 {
m.themePicker.cursor = 0
}
if m.themePicker.cursor >= count && count > 0 {
m.themePicker.cursor = count - 1
}

if m.themePicker.cursor < m.themePicker.scroll {
m.themePicker.scroll = m.themePicker.cursor
} else if m.themePicker.cursor >= m.themePicker.scroll+visible {
m.themePicker.scroll = m.themePicker.cursor - visible + 1
}

if m.themePicker.scroll+visible > count && count > 0 {
m.themePicker.scroll = max(0, count-visible)
}
}
Comment on lines +53 to +99
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial | ⚖️ Poor tradeoff

Theme-picker visibility/scroll helpers are correct but largely duplicate the keymap & file browser pattern.

themePickerVisible and themePickerMaybeAdjustScroll are near-line-for-line copies of keymapVisible/keymapMaybeAdjustScroll and fbVisible/fbMaybeAdjustScroll, only differing in the probe sections, the count source, and the cursor/scroll fields. With three overlays (and growing) reusing the same shape, a small generic helper — e.g. clampOverlayScroll(cursor, scroll *int, visible, count int) and a computeOverlayVisible(probeSections []string, heightExpanded bool, cap int) — would remove the drift risk if any of these handlers diverges.

Not blocking — just worth keeping in mind as the next overlay is added.

🤖 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 `@ui/model/overlays.go` around lines 53 - 99,
themePickerVisible/themePickerMaybeAdjustScroll duplicate logic from
keymapVisible/keymapMaybeAdjustScroll and fbVisible/fbMaybeAdjustScroll; extract
the shared behavior into two helpers and call them from each overlay: add
computeOverlayVisible(probeSections []string, heightExpanded bool, cap int) to
build the probe frame, compute fixedHeight and return the visible limit, and add
clampOverlayScroll(cursor *int, scroll *int, visible, count int) to normalize
cursor/scroll bounds and adjust scroll when cursor moves; then update
themePickerVisible to call computeOverlayVisible with its probeSections and
update themePickerMaybeAdjustScroll to call
clampOverlayScroll(&m.themePicker.cursor, &m.themePicker.scroll, visible,
len(m.themes)+1) (and similarly replace logic in keymap*/fb* to call these
helpers).


// openPlaylistManager loads playlist metadata and opens the manager overlay.
func (m *Model) openPlaylistManager() {
m.plMgrResetFilter()
Expand Down
1 change: 1 addition & 0 deletions ui/model/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ type seekState struct {
type themePickerState struct {
visible bool
cursor int
scroll int
savedIdx int // themeIdx before opening picker, for cancel/restore
}

Expand Down
22 changes: 10 additions & 12 deletions ui/model/view_overlays.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,9 @@ func (m Model) renderThemePicker() string {
}

count := len(m.themes) + 1
maxVisible := 15
scroll := scrollStart(m.themePicker.cursor, maxVisible)
maxVisible := m.themePickerVisible()
scroll := m.themePicker.scroll
rendered := 0

for i := scroll; i < count && i < scroll+maxVisible; i++ {
var name string
Expand All @@ -78,13 +79,12 @@ func (m Model) renderThemePicker() string {
name = m.themes[i-1].Name
}
lines = append(lines, cursorLine(name, i == m.themePicker.cursor))
rendered++
}

if count > maxVisible {
lines = append(lines, "", dimStyle.Render(fmt.Sprintf(" %d/%d themes", m.themePicker.cursor+1, count)))
}

lines = append(lines, "", helpKey("↓↑", "Scroll ")+helpKey("Enter", "Select ")+helpKey("Esc", "Cancel"))
lines = padLines(lines, maxVisible, rendered)
lines = append(lines, "", dimStyle.Render(fmt.Sprintf(" %d/%d themes", m.themePicker.cursor+1, count)))
lines = append(lines, "", m.themePickerHelpLine())

return m.centerOverlay(strings.Join(lines, "\n"))
}
Expand Down Expand Up @@ -496,6 +496,7 @@ func (m Model) renderURLInputOverlay() string {
}

func (m Model) renderLyricsOverlay() string {
visible := m.lyricsVisibleHeight()
lines := []string{
titleStyle.Render("L Y R I C S"),
"",
Expand Down Expand Up @@ -532,7 +533,6 @@ func (m Model) renderLyricsOverlay() string {
}
}

visible := max(m.height-8, 5)
half := visible / 2
startIdx := max(activeIdx-half, 0)
endIdx := startIdx + visible
Expand All @@ -554,7 +554,6 @@ func (m Model) renderLyricsOverlay() string {
}
} else {
// Scroll mode: manual navigation with j/k or arrow keys.
visible := max(m.height-8, 5)
endIdx := min(m.lyrics.scroll+visible, len(m.lyrics.lines))

for i := m.lyrics.scroll; i < endIdx; i++ {
Expand All @@ -566,9 +565,8 @@ func (m Model) renderLyricsOverlay() string {
}
}

for len(lines) < 14 {
lines = append(lines, "")
}
rendered := len(lines) - 2 // -2 for header and spacing
lines = padLines(lines, visible, rendered)

if m.lyricsSyncable() && m.lyricsHaveTimestamps() {
lines = append(lines, "", helpKey("Esc", "Close"))
Expand Down