Skip to content

Proposal: Adds ctrl+d and ctrl+u scrolling to search lists#262

Open
ReUhssurance wants to merge 1 commit into
bjarneo:mainfrom
ReUhssurance:search-enhancements
Open

Proposal: Adds ctrl+d and ctrl+u scrolling to search lists#262
ReUhssurance wants to merge 1 commit into
bjarneo:mainfrom
ReUhssurance:search-enhancements

Conversation

@ReUhssurance

@ReUhssurance ReUhssurance commented Jun 8, 2026

Copy link
Copy Markdown

Summary

When searching, j/k do not work to navigate lists, only arrow keys currently are supported. CTRL+D & CTRL+U keys are pretty standard for navigating up/down in lists. This adds those key maps in addition to the arrow keys for navigating search results.

Screenshots / video

screenrecording-2026-06-08_11-01-55.mp4

How to test

  1. make build && ./cliamp
  2. Open search
  3. Press ctrl+D/U to navigate results

Checklist

  • make check passes
  • docs/ and site/index.html updated for user-facing changes

Summary by CodeRabbit

  • New Features

    • Introduced Ctrl+U and Ctrl+D keyboard shortcuts for cursor navigation in search results, playlist panes, and search functionality.
  • Documentation

    • Updated keybinding documentation to reflect new navigation shortcuts.

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds Ctrl+U and Ctrl+D as cursor-navigation shortcuts across four search handlers: provider search, general search, network search results, and Spotify search results. Each handler implements wrap-around cursor movement (up/backward for Ctrl+U, down/forward for Ctrl+D) and adjusts scroll visibility. Documentation is updated to reflect these new bindings in the markdown and HTML keybinding reference tables.

Changes

Ctrl+U/Ctrl+D Navigation Shortcuts

Layer / File(s) Summary
Search handler navigation implementation
ui/model/keys.go, ui/model/keys_spotify_search.go
handleProvSearchKey, handleSearchKey, and handleNetSearchResultsKey each add ctrl+d and ctrl+u case handling to move cursors forward/backward through results with wrap-around and scroll adjustment. handleSpotSearchResultsKey adds ctrl+u to its upward navigation group.
Keybinding documentation updates
docs/keybindings.md, site/index.html
Markdown and HTML documentation tables are updated to include Ctrl+U / Ctrl+D alongside existing ↑/↓ and j/k shortcuts in playlist search, provider playlists pane, and search results sections.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The PR title references Ctrl+D and Ctrl+U, but the original objectives and PR description indicate the initial implementation used Ctrl+N and Ctrl+P, which were then changed to Ctrl+D/Ctrl+U per reviewer feedback. The title appears to reflect the final implementation, not the original proposal. Clarify the title to accurately reflect the current state of the PR or update it to 'Proposal: Adds ctrl+u and ctrl+d scrolling to search lists' if the changes have been finalized to use these keybindings.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
site/index.html (1)

1973-1973: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Document playlist search navigation shortcuts to match markdown.

The markdown documentation (docs/keybindings.md line 32) includes navigation instructions: "Search playlist (navigate results with / j k / Ctrl+N Ctrl+P)". The HTML version only mentions "Search playlist (fuzzy, ranked by relevance)" without any navigation documentation. As per coding guidelines, user-facing keybinding changes must update both docs/ and site/index.html in sync.

📝 Suggested update
-          <div class="key-row"><kbd>/</kbd><span>Search playlist (fuzzy, ranked by relevance)</span></div>
+          <div class="key-row"><kbd>/</kbd><span>Search playlist (fuzzy, ranked by relevance; navigate with &uarr;/&darr;, j/k, Ctrl+N/Ctrl+P)</span></div>
🤖 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 `@site/index.html` at line 1973, Update the site keybinding entry for the "/"
playlist search to match the markdown docs by adding the navigation hints shown
in docs/keybindings.md (navigate results with ↑ ↓ / j k / Ctrl+N Ctrl+P). Locate
the HTML element with class "key-row" containing <kbd>/</kbd> (the current span
text "Search playlist (fuzzy, ranked by relevance)") and extend its span content
to include the navigation instructions so both docs/keybindings.md and the
site/index.html entry are in sync.

Source: Coding guidelines

🤖 Prompt for all review comments with 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.

Outside diff comments:
In `@site/index.html`:
- Line 1973: Update the site keybinding entry for the "/" playlist search to
match the markdown docs by adding the navigation hints shown in
docs/keybindings.md (navigate results with ↑ ↓ / j k / Ctrl+N Ctrl+P). Locate
the HTML element with class "key-row" containing <kbd>/</kbd> (the current span
text "Search playlist (fuzzy, ranked by relevance)") and extend its span content
to include the navigation instructions so both docs/keybindings.md and the
site/index.html entry are in sync.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 045ed646-126e-4f61-b2ce-931260b1d56f

📥 Commits

Reviewing files that changed from the base of the PR and between 213ada6 and 1b9166f.

📒 Files selected for processing (5)
  • docs/keybindings.md
  • site/index.html
  • ui/model/keymap.go
  • ui/model/keys.go
  • ui/model/keys_spotify_search.go

@bjarneo

bjarneo commented Jun 8, 2026

Copy link
Copy Markdown
Owner

Hey. Would be nice to navigate using something like this, yes, and we have it already in playlists. If you change it to ctrl + d and ctrl + u I will happily accept this PR.

@ReUhssurance ReUhssurance force-pushed the search-enhancements branch from 1b9166f to fc35f35 Compare June 8, 2026 20:57
@ReUhssurance ReUhssurance changed the title Proposal: Adds ctrl+n and ctrl+p scrolling to search lists Proposal: Adds ctrl+d and ctrl+u scrolling to search lists Jun 8, 2026
@ReUhssurance

Copy link
Copy Markdown
Author

@bjarneo Sounds good to be more consistent if D/U are already used elsewhere. I just changed this PR to do that instead.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with 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.

Inline comments:
In `@docs/keybindings.md`:
- Line 119: The "Move cursor (wraps)" row incorrectly groups Ctrl+U/Ctrl+D with
single-row navigation; remove Ctrl+U and Ctrl+D from that table entry so it only
lists ↑/↓ and j/k, and ensure Ctrl+U/Ctrl+D remain documented under the
page-scroll entry and the search-results entry; reference the actual handlers
providerPageUp() and providerPageDown() (from keys.go) as the source of page
navigation behavior so the doc matches the implementation.

In `@site/index.html`:
- Line 2002: Remove the "Ctrl+U / Ctrl+D" keys from the key-row that currently
reads "↑ / ↓  j / k  Ctrl+U / Ctrl+D" so it only documents single-row movement
(↑/↓ and j/k) — do this by editing the key-row HTML that contains that combined
label. Ensure Ctrl+U and Ctrl+D remain documented as "Half-page scroll"
elsewhere (the Navigation section) and do not change
providerPageUp()/providerPageDown() behavior; confirm the new single-row
Ctrl+U/Ctrl+D mention stays only in the search-results documentation.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 92614d35-29f2-4c3a-aca5-96961d1ca109

📥 Commits

Reviewing files that changed from the base of the PR and between 1b9166f and fc35f35.

📒 Files selected for processing (4)
  • docs/keybindings.md
  • site/index.html
  • ui/model/keys.go
  • ui/model/keys_spotify_search.go

Comment thread docs/keybindings.md
| Key | Action |
|---|---|
| `↑` `↓` / `j` `k` | Move cursor (wraps) |
| `↑` `↓` / `j` `k` / `Ctrl+U` `Ctrl+D` | Move cursor (wraps) |

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Documentation inconsistency: Ctrl+U/Ctrl+D do page navigation, not single-row cursor movement in provider playlists pane.

In the provider playlists context, Ctrl+U and Ctrl+D trigger providerPageUp() and providerPageDown() (see ui/model/keys.go:348-352), which move the cursor by a full page (the visible height), not by a single row. Grouping them with / and j/k under "Move cursor (wraps)" is misleading.

These page-navigation keys are already correctly documented at line 27: "Scroll playlist/file browser by page". They should be removed from this row since they don't perform the same action as the single-row movement keys.

The new Ctrl+U/Ctrl+D single-row navigation added by this PR only applies to search result contexts (documented at line 136), not to the regular provider playlists browsing.

Suggested fix
-| `↑` `↓` / `j` `k` / `Ctrl+U` `Ctrl+D` | Move cursor (wraps) |
+| `↑` `↓` / `j` `k` | Move cursor (wraps) |
🤖 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 `@docs/keybindings.md` at line 119, The "Move cursor (wraps)" row incorrectly
groups Ctrl+U/Ctrl+D with single-row navigation; remove Ctrl+U and Ctrl+D from
that table entry so it only lists ↑/↓ and j/k, and ensure Ctrl+U/Ctrl+D remain
documented under the page-scroll entry and the search-results entry; reference
the actual handlers providerPageUp() and providerPageDown() (from keys.go) as
the source of page navigation behavior so the doc matches the implementation.

Comment thread site/index.html
<div class="keys-group-title">Provider Playlists Pane</div>
<div class="keys-group-body">
<div class="key-row"><kbd>↑ / ↓&nbsp;&nbsp;&nbsp;j / k</kbd><span>Move cursor (wraps)</span></div>
<div class="key-row"><kbd>↑ / ↓&nbsp;&nbsp;&nbsp;j / k&nbsp;&nbsp;&nbsp;Ctrl+U / Ctrl+D</kbd><span>Move cursor (wraps)</span></div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Documentation inconsistency: Ctrl+U/Ctrl+D do page navigation, not single-row cursor movement in provider playlists pane.

This line incorrectly groups Ctrl+U / Ctrl+D with single-row movement keys (↑ / ↓ and j / k) under "Move cursor (wraps)". In the provider playlists pane context, these keys trigger page navigation (providerPageUp() and providerPageDown() in ui/model/keys.go:348-352), not single-row movement.

Line 1961 already correctly documents them as "Half-page scroll" in the Navigation section. They should be removed from this row to avoid confusion.

The new Ctrl+U / Ctrl+D single-row navigation added by this PR only applies to search result contexts (documented at line 2031), not to regular provider playlists browsing.

Suggested fix
-          <div class="key-row"><kbd>↑ / ↓&nbsp;&nbsp;&nbsp;j / k&nbsp;&nbsp;&nbsp;Ctrl+U / Ctrl+D</kbd><span>Move cursor (wraps)</span></div>
+          <div class="key-row"><kbd>↑ / ↓&nbsp;&nbsp;&nbsp;j / k</kbd><span>Move cursor (wraps)</span></div>
📝 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
<div class="key-row"><kbd>↑ / ↓&nbsp;&nbsp;&nbsp;j / k&nbsp;&nbsp;&nbsp;Ctrl+U / Ctrl+D</kbd><span>Move cursor (wraps)</span></div>
<div class="key-row"><kbd>↑ / ↓&nbsp;&nbsp;&nbsp;j / k</kbd><span>Move cursor (wraps)</span></div>
🤖 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 `@site/index.html` at line 2002, Remove the "Ctrl+U / Ctrl+D" keys from the
key-row that currently reads "↑ / ↓  j / k  Ctrl+U / Ctrl+D" so it only
documents single-row movement (↑/↓ and j/k) — do this by editing the key-row
HTML that contains that combined label. Ensure Ctrl+U and Ctrl+D remain
documented as "Half-page scroll" elsewhere (the Navigation section) and do not
change providerPageUp()/providerPageDown() behavior; confirm the new single-row
Ctrl+U/Ctrl+D mention stays only in the search-results documentation.

@ReUhssurance

Copy link
Copy Markdown
Author

@bjarneo Those coderabbit comments got me thinking. Ctrl+D/U is used more for page down/up behavior. Ctrl+N/P is next/previous (single item) behavior. What do you think about adding both, and adding Ctrl+N/P where Ctrl+D/U is currently implemented? The way I just changed n/p -> d/u is next/previous and inconsistent with the behavior where D/U is used elsewhere.

I don't mean to expand the scope of such a small change but also want to be more consistent with other tooling in the broader scope of things.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants