fix(ui): change shuffle hotkey to "ctrl+t"#99
Conversation
📝 WalkthroughWalkthroughThe keyboard shortcut for toggling shuffle mode in the "I Feel Lucky" menu is changed from 't' to 'ctrl+t' across documentation, UI logic, and tests. This addresses a bug preventing users from typing text starting with 't' in the genre/keyword field. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
docs/README.md (1)
536-570:⚠️ Potential issue | 🟡 MinorShuffle shortcut table still lists
tinstead ofctrl+t.The updated steps now say
ctrl+t, but the shortcut table row remains outdated, which is confusing.Suggested patch
-| `t` | Toggle shuffle mode (in input screen) | +| `ctrl+t` | Toggle shuffle mode (in input screen) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/README.md` around lines 536 - 570, The "Shuffle Keyboard Shortcuts" table is inconsistent with the usage instructions: update the Key cell for the shuffle toggle from `t` to `ctrl+t` so it matches the steps that say "Press `ctrl+t` to toggle shuffle mode"; ensure the Action cell still reads "Toggle shuffle mode (in input screen)" and verify any other references to the `t` shortcut in the README (e.g., the "Shuffle Keyboard Shortcuts" heading or nearby descriptions) are changed to `ctrl+t` for consistency.README.md (1)
536-570:⚠️ Potential issue | 🟡 MinorShortcut reference is inconsistent: table still shows
tfor shuffle toggle.Please update the Shuffle Keyboard Shortcuts table to match the new
ctrl+tbinding.Suggested patch
-| `t` | Toggle shuffle mode (in input screen) | +| `ctrl+t` | Toggle shuffle mode (in input screen) |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` around lines 536 - 570, Update the "Shuffle Keyboard Shortcuts" table in README.md so the shuffle toggle row reflects the new binding: replace the `t` entry with `ctrl+t` (keep the Action text "Toggle shuffle mode (in input screen)" unchanged) and ensure any other mentions of `t` for toggling shuffle in that section are updated to `ctrl+t`.internal/ui/lucky.go (1)
1021-1061:⚠️ Potential issue | 🟡 MinorBottom help text is stale and still says
tinstead ofctrl+t.Line 1060 currently advertises
t: Toggle shuffle, which conflicts with Line 451 behavior and the updated inline hints.Suggested patch
- helpText := "↑↓/jk: Navigate • Enter: Search • t: Toggle shuffle" + helpText := "↑↓/jk: Navigate • Enter: Search • ctrl+t: Toggle shuffle"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/ui/lucky.go` around lines 1021 - 1061, The bottom helpText string currently says "t: Toggle shuffle" but the UI uses ctrl+t to toggle shuffle (see use of m.shuffleEnabled and inline hints showing "ctrl+t"); update the helpText variable to advertise "ctrl+t: Toggle shuffle" (or "ctrl+t/ctrl+T" as desired) so it matches the actual toggle behavior and the inline hints rendered for m.shuffleEnabled and the shuffle toggle handler.
🧹 Nitpick comments (1)
internal/ui/lucky_test.go (1)
565-597: Add a regression test for the original bug path ("trance"should type normally).Current test covers toggling, but it doesn’t assert that typing a leading
tno longer toggles shuffle while entering text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/ui/lucky_test.go` around lines 565 - 597, Update the TestLuckyShuffleToggle to include a regression check that typing a leading "t" (e.g., sending a tea.KeyMsg for rune 't') inserts the character into model.textInput instead of toggling shuffle; after creating NewLuckyModel and setting model.state = luckyStateInput, send a rune KeyMsg for 't', call model.Update (or luckyModel.Update) and assert that shuffleEnabled remains unchanged and textInput.Value() becomes "t" (and similarly verify typing "trance" produces "trance"), referencing TestLuckyShuffleToggle, LuckyModel, Update, shuffleEnabled, textInput and luckyStateInput to locate the logic to test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/ui/lucky_test.go`:
- Line 577: Replace the synthetic rune-based key messages with proper control
key events: change constructions like tea.KeyMsg{Type: tea.KeyRunes, Runes:
[]rune("ctrl+t")} (and the other occurrence at the second spot) to use the
Bubble Tea control key constant tea.KeyCtrlT by creating tea.KeyMsg{Type:
tea.KeyCtrlT}; update both instances in the test (the variable msg assignments)
so the test sends a real Ctrl+T key event instead of rune characters.
---
Outside diff comments:
In `@docs/README.md`:
- Around line 536-570: The "Shuffle Keyboard Shortcuts" table is inconsistent
with the usage instructions: update the Key cell for the shuffle toggle from `t`
to `ctrl+t` so it matches the steps that say "Press `ctrl+t` to toggle shuffle
mode"; ensure the Action cell still reads "Toggle shuffle mode (in input
screen)" and verify any other references to the `t` shortcut in the README
(e.g., the "Shuffle Keyboard Shortcuts" heading or nearby descriptions) are
changed to `ctrl+t` for consistency.
In `@internal/ui/lucky.go`:
- Around line 1021-1061: The bottom helpText string currently says "t: Toggle
shuffle" but the UI uses ctrl+t to toggle shuffle (see use of m.shuffleEnabled
and inline hints showing "ctrl+t"); update the helpText variable to advertise
"ctrl+t: Toggle shuffle" (or "ctrl+t/ctrl+T" as desired) so it matches the
actual toggle behavior and the inline hints rendered for m.shuffleEnabled and
the shuffle toggle handler.
In `@README.md`:
- Around line 536-570: Update the "Shuffle Keyboard Shortcuts" table in
README.md so the shuffle toggle row reflects the new binding: replace the `t`
entry with `ctrl+t` (keep the Action text "Toggle shuffle mode (in input
screen)" unchanged) and ensure any other mentions of `t` for toggling shuffle in
that section are updated to `ctrl+t`.
---
Nitpick comments:
In `@internal/ui/lucky_test.go`:
- Around line 565-597: Update the TestLuckyShuffleToggle to include a regression
check that typing a leading "t" (e.g., sending a tea.KeyMsg for rune 't')
inserts the character into model.textInput instead of toggling shuffle; after
creating NewLuckyModel and setting model.state = luckyStateInput, send a rune
KeyMsg for 't', call model.Update (or luckyModel.Update) and assert that
shuffleEnabled remains unchanged and textInput.Value() becomes "t" (and
similarly verify typing "trance" produces "trance"), referencing
TestLuckyShuffleToggle, LuckyModel, Update, shuffleEnabled, textInput and
luckyStateInput to locate the logic to test.
|
|
||
| // Toggle shuffle on | ||
| msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("t")} | ||
| msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("ctrl+t")} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n internal/ui/lucky_test.go | sed -n '570,600p'Repository: shinokada/tera
Length of output: 1118
🏁 Script executed:
rg -n 'tea\.Key' internal/ui/lucky_test.go | head -20Repository: shinokada/tera
Length of output: 977
🏁 Script executed:
rg -n 'tea\.KeyCtrlT|tea\.KeyRunes' internal/ -t goRepository: shinokada/tera
Length of output: 3740
🏁 Script executed:
rg -i 'keyctrl' internal/ -t goRepository: shinokada/tera
Length of output: 40
🌐 Web query:
Bubble Tea tea.Key control key types tea.KeyCtrlT API
💡 Result:
In Bubble Tea (tea), control keys are represented by tea.KeyType constants. tea.KeyCtrlT is the key type for Ctrl+T (it’s an alias of the control code keyDC4). [1]
To handle it, match on tea.KeyMsg in Update:
case tea.KeyMsg:
switch {
case msg.Type == tea.KeyCtrlT:
// Ctrl+T pressed
// or:
case msg.String() == "ctrl+t":
// Ctrl+T pressed
}msg.String() is "ctrl+t" for that key. [2]
Sources: [1] [2]
Replace tea.KeyRunes with tea.KeyCtrlT for control key events in tests.
tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("ctrl+t")} creates synthetic rune characters instead of a Ctrl+T key event. Use tea.KeyMsg{Type: tea.KeyCtrlT} to properly represent control key presses in Bubble Tea.
Suggested patch
- msg := tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("ctrl+t")}
+ msg := tea.KeyMsg{Type: tea.KeyCtrlT}Also applies to: 590-590
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/ui/lucky_test.go` at line 577, Replace the synthetic rune-based key
messages with proper control key events: change constructions like
tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("ctrl+t")} (and the other
occurrence at the second spot) to use the Bubble Tea control key constant
tea.KeyCtrlT by creating tea.KeyMsg{Type: tea.KeyCtrlT}; update both instances
in the test (the variable msg assignments) so the test sends a real Ctrl+T key
event instead of rune characters.
|
I just found another issue. Entering any number for the Genre/keyword, for example I'm not sure how to get around that without some redesigning. |
fix: #99 * **Documentation** * Updated shuffle shortcut from `t` to `Ctrl+T` across guides and help text. * **User Interface** * In-app hints and messages now reference `Ctrl+T` for toggling shuffle. * **Bug Fixes** * Number-key entry now only accumulates when not typing into the text input, improving input behavior. * **Tests** * Updated and added tests to validate `Ctrl+T` behavior and non-toggle-while-typing cases. * **Chores** * Build task now runs tests before compiling.
1 participant
Closes #97
📑 Description
This change modifies the Shuffle mode toggle hot key in the I Feel Lucky menu from
ttoctrl+t. This allows the user to type any value in the Genre/keyword field without triggering the Shuffle mode toggle when it begins with at.The alternative is to make Shuffle mode a selectable text input that toggles the
✓off and on when the user presses space/enter/etc. This would require more changes to allow navigating up and down the two input fields and through the list of Recent Searches. Currently, up and down is exclusively for the Recent Searches.However, this change is the fast and minimal amount of changes. The hotkey can be changed, preferably a
ctrl,meta, oraltkey combination to avoid erroneously toggling.✅ Checks
ℹ Additional Information
Summary by CodeRabbit
Documentation
Updates