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
23 changes: 18 additions & 5 deletions authbridge/cmd/abctl/tui/events_pane.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,11 +154,24 @@ func (m *model) rebuildEventsTable() {

// Auto-follow: if user was at the bottom, stay at the bottom. Otherwise
// preserve position so reading isn't disturbed by new events.
if wasAtEnd && len(rows) > 0 {
m.eventsTbl.SetCursor(len(rows) - 1)
} else if prevRow < len(rows) {
m.eventsTbl.SetCursor(prevRow)
}
//
// Unconditional now, and through setCursorVisible rather than SetCursor. Two
// reasons, both about the highlight rather than the index:
//
// - SetCursor does not reconcile the viewport's offset, so restoring any row
// at or past one screenful left the cursor one line below the rendered
// window. On a live session that meant the highlight disappeared on the very
// next event, for every session long enough to scroll. setCursorVisible
// carries the mechanics.
// - the old `else if prevRow < len(rows)` skipped the restore entirely when
// the rows SHRANK under the cursor — a filter typed, hideInactive toggled.
// SetRows had clamped the index by then, so the cursor was left wherever
// that landed, with an offset nobody reconciled.
target := prevRow
if wasAtEnd {
target = len(rows) - 1
}
setCursorVisible(&m.eventsTbl, target)
}

// selectedEvent returns the event at the cursor row, or nil. The cursor
Expand Down
37 changes: 18 additions & 19 deletions authbridge/cmd/abctl/tui/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -605,12 +605,14 @@ func (m *model) handleKey(msg tea.KeyMsg) tea.Cmd {
prev := m.pipelineTbl.Cursor()
var cmd tea.Cmd
m.pipelineTbl, cmd = m.pipelineTbl.Update(msg)
// Skip over the divider row when navigating.
// Skip over the divider row when navigating. One more step in the direction
// of travel, as a relative move so the offset stays reconciled — see
// setCursorVisible for why SetCursor is not used for cursor placement.
if isDividerRow(m.pipelineTbl.Rows(), m.pipelineTbl.Cursor()) {
if m.pipelineTbl.Cursor() > prev {
m.pipelineTbl.SetCursor(m.pipelineTbl.Cursor() + 1)
m.pipelineTbl.MoveDown(1)
} else {
m.pipelineTbl.SetCursor(m.pipelineTbl.Cursor() - 1)
m.pipelineTbl.MoveUp(1)
}
}
return cmd
Expand Down Expand Up @@ -640,16 +642,21 @@ func (m *model) refreshActivePane() {
}
}

// goTop and goBottom place the cursor through setCursorVisible, not SetCursor: a
// jump to the last row is exactly the case where SetCursor leaves the highlight one
// line below the rendered window, so `G` on any list longer than the screen used to
// scroll to the bottom with nothing highlighted. The empty-table guards live in
// setCursorVisible now, and it clamps, so goBottom does not need the row count.
func (m *model) goTop() {
switch m.pane {
case paneCatalog:
m.catalogTbl.SetCursor(0)
setCursorVisible(&m.catalogTbl, 0)
case paneSessions:
m.sessionsTbl.SetCursor(0)
setCursorVisible(&m.sessionsTbl, 0)
case paneEvents:
m.eventsTbl.SetCursor(0)
setCursorVisible(&m.eventsTbl, 0)
case panePipeline:
m.pipelineTbl.SetCursor(0)
setCursorVisible(&m.pipelineTbl, 0)
case paneDetail, panePluginDetail:
m.detailVp.GotoTop()
}
Expand All @@ -658,21 +665,13 @@ func (m *model) goTop() {
func (m *model) goBottom() {
switch m.pane {
case paneSessions:
if n := len(m.sessionsTbl.Rows()); n > 0 {
m.sessionsTbl.SetCursor(n - 1)
}
setCursorVisible(&m.sessionsTbl, len(m.sessionsTbl.Rows())-1)
case paneEvents:
if n := len(m.eventsTbl.Rows()); n > 0 {
m.eventsTbl.SetCursor(n - 1)
}
setCursorVisible(&m.eventsTbl, len(m.eventsTbl.Rows())-1)
case panePipeline:
if n := len(m.pipelineTbl.Rows()); n > 0 {
m.pipelineTbl.SetCursor(n - 1)
}
setCursorVisible(&m.pipelineTbl, len(m.pipelineTbl.Rows())-1)
case paneCatalog:
if n := len(m.catalogTbl.Rows()); n > 0 {
m.catalogTbl.SetCursor(n - 1)
}
setCursorVisible(&m.catalogTbl, len(m.catalogTbl.Rows())-1)
case paneDetail, panePluginDetail:
m.detailVp.GotoBottom()
}
Expand Down
5 changes: 3 additions & 2 deletions authbridge/cmd/abctl/tui/pipeline_pane.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ func (m *model) rebuildPipelineTable() {
rows = append(rows, pipelineRow(p, counts[p.Name], m.pipeline.Outbound))
}
m.pipelineTbl.SetRows(rows)
// If cursor is on the divider row, nudge to the next plugin.
// If cursor is on the divider row, nudge to the next plugin. A relative move so
// the viewport offset stays reconciled — see setCursorVisible.
if isDividerRow(rows, m.pipelineTbl.Cursor()) {
m.pipelineTbl.SetCursor(m.pipelineTbl.Cursor() + 1)
m.pipelineTbl.MoveDown(1)
}
}

Expand Down
10 changes: 5 additions & 5 deletions authbridge/cmd/abctl/tui/sessions_pane.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,18 @@ func (m *model) rebuildSessionsTable() {
}
m.sessionsTbl.SetRows(rows)

// Restore cursor position if possible.
// Restore cursor position if possible. Through setCursorVisible: a restored row
// past the first screenful would otherwise land one line below the rendered
// window, leaving the pane with no highlight — see setCursorVisible.
if prev != "" {
for i, r := range rows {
if r[0] == prev {
m.sessionsTbl.SetCursor(i)
setCursorVisible(&m.sessionsTbl, i)
return
}
}
}
if len(rows) > 0 {
m.sessionsTbl.SetCursor(0)
}
setCursorVisible(&m.sessionsTbl, 0)
}

// cachedOnlySessionIDs lists sessions abctl has events for that the server's
Expand Down
47 changes: 47 additions & 0 deletions authbridge/cmd/abctl/tui/table_cursor.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package tui

import "github.com/charmbracelet/bubbles/table"

// setCursorVisible moves a table's cursor to row n and leaves it ON SCREEN.
//
// Use this instead of table.SetCursor everywhere a cursor is placed
// programmatically — following a tail, restoring a saved position, jumping to an
// end. SetCursor alone loses the highlight for any row at or past one screenful.
//
// WHY: bubbles' UpdateViewport renders a WINDOW of rows around the cursor —
// start = clamp(cursor−height, 0, cursor) — and the viewport then shows `height`
// lines of that window beginning at its own YOffset. SetCursor recomputes the
// window but never reconciles YOffset, and only MoveUp/MoveDown do. So with
// YOffset 0 and cursor ≥ height, start lands on cursor−height and the cursor sits
// at window line `height`: exactly one line past the last visible one. The row is
// rendered, the highlight is drawn on it, and it is off the bottom edge.
//
// MoveUp cannot recover from that state either — its three cases are start == 0,
// start < height, and YOffset ≥ 1, and in that state none of them match — so YOffset
// stays 0 while start walks up with the cursor. The rows scroll one at a time under an
// arrow key and no row is ever highlighted, which is how this reached us: "the
// lines are getting scrolled up, but the highlight disappears".
//
// The fix is to express the jump as relative movement, the only cursor API in
// bubbles that maintains the offset: GotoTop normalizes it (a MoveUp to row 0),
// then one MoveDown of n walks to the target. n is a distance, not a loop, so this
// is two O(height) re-renders however far the cursor travels.
//
// n is clamped to the row range, as SetCursor's own is. An empty table is left
// alone rather than parked on a row that does not exist.
func setCursorVisible(t *table.Model, n int) {
rows := len(t.Rows())
if rows == 0 {
return
}
if n < 0 {
n = 0
}
if n > rows-1 {
n = rows - 1
}
t.GotoTop()
if n > 0 {
t.MoveDown(n)
}
}
Loading
Loading