Skip to content

Commit 49dc4fb

Browse files
wesmclaude
andauthored
Fix TUI left/right arrow navigation direction (#451)
## Summary - Rename all `findPrev*`/`findNext*` nav functions so Prev = older (lower ID, higher array index) and Next = newer (higher ID, lower array index), matching the `id DESC` sort order of the queue - Swap `handlePrevKey`/`handleNextKey` definitions to match the corrected naming - Update key bindings so `j`/`left` → prev (older) and `k`/`right` → next (newer) - Update pagination auto-navigate calls in `handlers_msg.go` to use renamed functions - Update all arrow key test expectations across queue, review navigation, and log navigation tests 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 2ea9048 commit 49dc4fb

8 files changed

Lines changed: 178 additions & 124 deletions

File tree

cmd/roborev/tui/action_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -393,10 +393,10 @@ func TestTUIClosedToggleMovesSelectionWithHideActive(t *testing.T) {
393393
}
394394

395395
// Simulate what happens in 'a' handler - selection should move
396-
// Since job 2 is now hidden, find next visible
397-
nextIdx := m.findNextVisibleJob(m.selectedIdx)
398-
if nextIdx != 2 {
399-
t.Errorf("Expected next visible job at index 2, got %d", nextIdx)
396+
// Since job 2 is now hidden, find prev (older) visible job
397+
prevIdx := m.findPrevVisibleJob(m.selectedIdx)
398+
if prevIdx != 2 {
399+
t.Errorf("Expected prev visible job at index 2, got %d", prevIdx)
400400
}
401401
}
402402

cmd/roborev/tui/handlers.go

Lines changed: 50 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -81,11 +81,11 @@ func (m model) handleGlobalKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) {
8181
return m.handleHomeKey()
8282
case "up":
8383
return m.handleUpKey()
84-
case "k", "left":
84+
case "j", "left":
8585
return m.handlePrevKey()
8686
case "down":
8787
return m.handleDownKey()
88-
case "j", "right":
88+
case "k", "right":
8989
return m.handleNextKey()
9090
case "pgup":
9191
return m.handlePageUpKey()
@@ -197,9 +197,9 @@ func (m model) handleHomeKey() (tea.Model, tea.Cmd) {
197197
func (m model) handleUpKey() (tea.Model, tea.Cmd) {
198198
switch m.currentView {
199199
case viewQueue:
200-
prevIdx := m.findPrevVisibleJob(m.selectedIdx)
201-
if prevIdx >= 0 {
202-
m.selectedIdx = prevIdx
200+
nextIdx := m.findNextVisibleJob(m.selectedIdx)
201+
if nextIdx >= 0 {
202+
m.selectedIdx = nextIdx
203203
m.updateSelectedJobID()
204204
} else {
205205
m.setFlash("No newer review", 2*time.Second, viewQueue)
@@ -224,22 +224,22 @@ func (m model) handleUpKey() (tea.Model, tea.Cmd) {
224224
return m, nil
225225
}
226226

227-
func (m model) handlePrevKey() (tea.Model, tea.Cmd) {
227+
func (m model) handleNextKey() (tea.Model, tea.Cmd) {
228228
switch m.currentView {
229229
case viewQueue:
230-
prevIdx := m.findPrevVisibleJob(m.selectedIdx)
231-
if prevIdx >= 0 {
232-
m.selectedIdx = prevIdx
230+
nextIdx := m.findNextVisibleJob(m.selectedIdx)
231+
if nextIdx >= 0 {
232+
m.selectedIdx = nextIdx
233233
m.updateSelectedJobID()
234234
}
235235
case viewReview:
236-
prevIdx := m.findPrevViewableJob()
237-
if prevIdx >= 0 {
236+
nextIdx := m.findNextViewableJob()
237+
if nextIdx >= 0 {
238238
m.closeFixPanel()
239-
m.selectedIdx = prevIdx
239+
m.selectedIdx = nextIdx
240240
m.updateSelectedJobID()
241241
m.reviewScroll = 0
242-
job := m.jobs[prevIdx]
242+
job := m.jobs[nextIdx]
243243
switch job.Status {
244244
case storage.JobStatusDone:
245245
return m, m.fetchReview(job.ID)
@@ -255,12 +255,12 @@ func (m model) handlePrevKey() (tea.Model, tea.Cmd) {
255255
m.setFlash("No newer review", 2*time.Second, viewReview)
256256
}
257257
case viewKindPrompt:
258-
prevIdx := m.findPrevPromptableJob()
259-
if prevIdx >= 0 {
260-
m.selectedIdx = prevIdx
258+
nextIdx := m.findNextPromptableJob()
259+
if nextIdx >= 0 {
260+
m.selectedIdx = nextIdx
261261
m.updateSelectedJobID()
262262
m.promptScroll = 0
263-
job := m.jobs[prevIdx]
263+
job := m.jobs[nextIdx]
264264
if job.Status == storage.JobStatusDone {
265265
return m, m.fetchReviewForPrompt(job.ID)
266266
} else if job.Status == storage.JobStatusRunning && job.Prompt != "" {
@@ -275,7 +275,7 @@ func (m model) handlePrevKey() (tea.Model, tea.Cmd) {
275275
}
276276
case viewLog:
277277
if m.logFromView == viewTasks {
278-
idx := m.findPrevLoggableFixJob()
278+
idx := m.findNextLoggableFixJob()
279279
if idx >= 0 {
280280
m.fixSelectedIdx = idx
281281
job := m.fixJobs[idx]
@@ -285,11 +285,11 @@ func (m model) handlePrevKey() (tea.Model, tea.Cmd) {
285285
)
286286
}
287287
} else {
288-
prevIdx := m.findPrevLoggableJob()
289-
if prevIdx >= 0 {
290-
m.selectedIdx = prevIdx
288+
nextIdx := m.findNextLoggableJob()
289+
if nextIdx >= 0 {
290+
m.selectedIdx = nextIdx
291291
m.updateSelectedJobID()
292-
job := m.jobs[prevIdx]
292+
job := m.jobs[nextIdx]
293293
m.logStreaming = false
294294
return m.openLogView(
295295
job.ID, job.Status, m.logFromView,
@@ -304,11 +304,11 @@ func (m model) handlePrevKey() (tea.Model, tea.Cmd) {
304304
func (m model) handleDownKey() (tea.Model, tea.Cmd) {
305305
switch m.currentView {
306306
case viewQueue:
307-
nextIdx := m.findNextVisibleJob(m.selectedIdx)
308-
if nextIdx >= 0 {
309-
m.selectedIdx = nextIdx
307+
prevIdx := m.findPrevVisibleJob(m.selectedIdx)
308+
if prevIdx >= 0 {
309+
m.selectedIdx = prevIdx
310310
m.updateSelectedJobID()
311-
if cmd := m.maybePrefetch(nextIdx); cmd != nil {
311+
if cmd := m.maybePrefetch(prevIdx); cmd != nil {
312312
return m, cmd
313313
}
314314
} else if m.canPaginate() {
@@ -335,28 +335,28 @@ func (m model) handleDownKey() (tea.Model, tea.Cmd) {
335335
return m, nil
336336
}
337337

338-
func (m model) handleNextKey() (tea.Model, tea.Cmd) {
338+
func (m model) handlePrevKey() (tea.Model, tea.Cmd) {
339339
switch m.currentView {
340340
case viewQueue:
341-
nextIdx := m.findNextVisibleJob(m.selectedIdx)
342-
if nextIdx >= 0 {
343-
m.selectedIdx = nextIdx
341+
prevIdx := m.findPrevVisibleJob(m.selectedIdx)
342+
if prevIdx >= 0 {
343+
m.selectedIdx = prevIdx
344344
m.updateSelectedJobID()
345-
if cmd := m.maybePrefetch(nextIdx); cmd != nil {
345+
if cmd := m.maybePrefetch(prevIdx); cmd != nil {
346346
return m, cmd
347347
}
348348
} else if m.canPaginate() {
349349
m.loadingMore = true
350350
return m, m.fetchMoreJobs()
351351
}
352352
case viewReview:
353-
nextIdx := m.findNextViewableJob()
354-
if nextIdx >= 0 {
353+
prevIdx := m.findPrevViewableJob()
354+
if prevIdx >= 0 {
355355
m.closeFixPanel()
356-
m.selectedIdx = nextIdx
356+
m.selectedIdx = prevIdx
357357
m.updateSelectedJobID()
358358
m.reviewScroll = 0
359-
job := m.jobs[nextIdx]
359+
job := m.jobs[prevIdx]
360360
switch job.Status {
361361
case storage.JobStatusDone:
362362
return m, m.fetchReview(job.ID)
@@ -376,12 +376,12 @@ func (m model) handleNextKey() (tea.Model, tea.Cmd) {
376376
m.setFlash("No older review", 2*time.Second, viewReview)
377377
}
378378
case viewKindPrompt:
379-
nextIdx := m.findNextPromptableJob()
380-
if nextIdx >= 0 {
381-
m.selectedIdx = nextIdx
379+
prevIdx := m.findPrevPromptableJob()
380+
if prevIdx >= 0 {
381+
m.selectedIdx = prevIdx
382382
m.updateSelectedJobID()
383383
m.promptScroll = 0
384-
job := m.jobs[nextIdx]
384+
job := m.jobs[prevIdx]
385385
if job.Status == storage.JobStatusDone {
386386
return m, m.fetchReviewForPrompt(job.ID)
387387
} else if job.Status == storage.JobStatusRunning && job.Prompt != "" {
@@ -400,7 +400,7 @@ func (m model) handleNextKey() (tea.Model, tea.Cmd) {
400400
}
401401
case viewLog:
402402
if m.logFromView == viewTasks {
403-
idx := m.findNextLoggableFixJob()
403+
idx := m.findPrevLoggableFixJob()
404404
if idx >= 0 {
405405
m.fixSelectedIdx = idx
406406
job := m.fixJobs[idx]
@@ -410,11 +410,11 @@ func (m model) handleNextKey() (tea.Model, tea.Cmd) {
410410
)
411411
}
412412
} else {
413-
nextIdx := m.findNextLoggableJob()
414-
if nextIdx >= 0 {
415-
m.selectedIdx = nextIdx
413+
prevIdx := m.findPrevLoggableJob()
414+
if prevIdx >= 0 {
415+
m.selectedIdx = prevIdx
416416
m.updateSelectedJobID()
417-
job := m.jobs[nextIdx]
417+
job := m.jobs[prevIdx]
418418
m.logStreaming = false
419419
return m.openLogView(
420420
job.ID, job.Status, m.logFromView,
@@ -435,11 +435,11 @@ func (m model) handlePageUpKey() (tea.Model, tea.Cmd) {
435435
switch m.currentView {
436436
case viewQueue:
437437
for range pageSize {
438-
prevIdx := m.findPrevVisibleJob(m.selectedIdx)
439-
if prevIdx < 0 {
438+
nextIdx := m.findNextVisibleJob(m.selectedIdx)
439+
if nextIdx < 0 {
440440
break
441441
}
442-
m.selectedIdx = prevIdx
442+
m.selectedIdx = nextIdx
443443
}
444444
m.updateSelectedJobID()
445445
case viewReview:
@@ -466,12 +466,12 @@ func (m model) handlePageDownKey() (tea.Model, tea.Cmd) {
466466
case viewQueue:
467467
reachedEnd := false
468468
for range pageSize {
469-
nextIdx := m.findNextVisibleJob(m.selectedIdx)
470-
if nextIdx < 0 {
469+
prevIdx := m.findPrevVisibleJob(m.selectedIdx)
470+
if prevIdx < 0 {
471471
reachedEnd = true
472472
break
473473
}
474-
m.selectedIdx = nextIdx
474+
m.selectedIdx = prevIdx
475475
}
476476
m.updateSelectedJobID()
477477
if reachedEnd && m.canPaginate() {

cmd/roborev/tui/handlers_msg.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func (m model) handleJobsMsg(msg jobsMsg) (tea.Model, tea.Cmd) {
163163
m.paginateNav = 0
164164
switch nav {
165165
case viewReview:
166-
nextIdx := m.findNextViewableJob()
166+
nextIdx := m.findPrevViewableJob()
167167
if nextIdx >= 0 {
168168
m.selectedIdx = nextIdx
169169
m.updateSelectedJobID()
@@ -182,7 +182,7 @@ func (m model) handleJobsMsg(msg jobsMsg) (tea.Model, tea.Cmd) {
182182
}
183183
}
184184
case viewKindPrompt:
185-
nextIdx := m.findNextPromptableJob()
185+
nextIdx := m.findPrevPromptableJob()
186186
if nextIdx >= 0 {
187187
m.selectedIdx = nextIdx
188188
m.updateSelectedJobID()
@@ -199,7 +199,7 @@ func (m model) handleJobsMsg(msg jobsMsg) (tea.Model, tea.Cmd) {
199199
}
200200
}
201201
case viewLog:
202-
nextIdx := m.findNextLoggableJob()
202+
nextIdx := m.findPrevLoggableJob()
203203
if nextIdx >= 0 {
204204
m.selectedIdx = nextIdx
205205
m.updateSelectedJobID()

cmd/roborev/tui/handlers_review.go

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -72,15 +72,15 @@ func (m model) handleCloseKey() (tea.Model, tea.Cmd) {
7272
m.pendingClosed[job.ID] = pendingState{newState: newState, seq: seq}
7373
m.applyStatsDelta(newState)
7474
if m.hideClosed && newState {
75-
nextIdx := m.findNextVisibleJob(m.selectedIdx)
76-
if nextIdx < 0 {
77-
nextIdx = m.findPrevVisibleJob(m.selectedIdx)
75+
idx := m.findPrevVisibleJob(m.selectedIdx)
76+
if idx < 0 {
77+
idx = m.findNextVisibleJob(m.selectedIdx)
7878
}
79-
if nextIdx < 0 {
80-
nextIdx = m.findFirstVisibleJob()
79+
if idx < 0 {
80+
idx = m.findFirstVisibleJob()
8181
}
82-
if nextIdx >= 0 {
83-
m.selectedIdx = nextIdx
82+
if idx >= 0 {
83+
m.selectedIdx = idx
8484
m.updateSelectedJobID()
8585
}
8686
}
@@ -103,15 +103,15 @@ func (m model) handleCancelKey() (tea.Model, tea.Cmd) {
103103
job.FinishedAt = &now
104104
// Canceled jobs are hidden when hideClosed is active
105105
if m.hideClosed {
106-
nextIdx := m.findNextVisibleJob(m.selectedIdx)
107-
if nextIdx < 0 {
108-
nextIdx = m.findPrevVisibleJob(m.selectedIdx)
106+
idx := m.findPrevVisibleJob(m.selectedIdx)
107+
if idx < 0 {
108+
idx = m.findNextVisibleJob(m.selectedIdx)
109109
}
110-
if nextIdx < 0 {
111-
nextIdx = m.findFirstVisibleJob()
110+
if idx < 0 {
111+
idx = m.findFirstVisibleJob()
112112
}
113-
if nextIdx >= 0 {
114-
m.selectedIdx = nextIdx
113+
if idx >= 0 {
114+
m.selectedIdx = idx
115115
m.updateSelectedJobID()
116116
}
117117
}

0 commit comments

Comments
 (0)