Act on the selected row, not the one the table moved to
In Users, pressing k on a user opened API keys for the user above it. The dashboard hands every key to the section's table before the section's own handler reads the cursor, and bubbles' table claims several letters for navigation: k is up, d half a page down, f a page down. A letter that is also an action therefore moved the cursor first, and the action landed on the row it had moved to. With your own user first in the list, k looked like it only ever showed your own keys. The same collision hit two destructive keys: - d in Users asked to delete a user half a page below the selected one. The confirmation names the user, and that was the only thing standing between a keypress and deleting the wrong person. - d in Schedule targeted a different day's assignment the same way. f in Incidents and Alerts cycled the filter and paged the cursor down too, which was harmless but wrong. Each table now gives up exactly the letters its section acts on, through tableKeyMap. The arrow keys and every other default binding are untouched. The cost is that k no longer moves up in Users, where it means API keys, as the README has always said. The up arrow still works, and the README now says to use it there. users_test.go reproduces all four. With tableKeyMap reverted to the defaults, each of them fails exactly as reported.
This commit is contained in:
@@ -193,3 +193,6 @@ Users section:
|
|||||||
| `t` | Edit the user's ntfy topic — submit empty to clear it |
|
| `t` | Edit the user's ntfy topic — submit empty to clear it |
|
||||||
| `d` | Delete a user |
|
| `d` | Delete a user |
|
||||||
| `k` | API keys for the selected user |
|
| `k` | API keys for the selected user |
|
||||||
|
|
||||||
|
In Users, `k` and `d` act on the selected row, so move with `↑`/`↓` there rather
|
||||||
|
than `k`.
|
||||||
|
|||||||
+37
-6
@@ -2,11 +2,13 @@ package tui
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"slices"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"git.ryuvia.com/niklas/terdut-tui/internal/api"
|
"git.ryuvia.com/niklas/terdut-tui/internal/api"
|
||||||
"git.ryuvia.com/niklas/terdut-tui/internal/theme"
|
"git.ryuvia.com/niklas/terdut-tui/internal/theme"
|
||||||
"github.com/charmbracelet/bubbles/help"
|
"github.com/charmbracelet/bubbles/help"
|
||||||
|
"github.com/charmbracelet/bubbles/key"
|
||||||
"github.com/charmbracelet/bubbles/table"
|
"github.com/charmbracelet/bubbles/table"
|
||||||
"github.com/charmbracelet/bubbles/textinput"
|
"github.com/charmbracelet/bubbles/textinput"
|
||||||
"github.com/charmbracelet/bubbles/viewport"
|
"github.com/charmbracelet/bubbles/viewport"
|
||||||
@@ -253,22 +255,26 @@ func NewModel(client *api.Client, serverURL string, refreshInterval time.Duratio
|
|||||||
st := newStyles(th)
|
st := newStyles(th)
|
||||||
ts := st.Table()
|
ts := st.Table()
|
||||||
|
|
||||||
incidentT := table.New(table.WithFocused(true))
|
// Each table sees a key before the section's own handler does, so any
|
||||||
|
// key a section uses as an action must be taken out of that table's
|
||||||
|
// navigation bindings, or the cursor moves first and the action lands on
|
||||||
|
// a different row. See tableKeyMap.
|
||||||
|
incidentT := table.New(table.WithFocused(true), table.WithKeyMap(tableKeyMap("f")))
|
||||||
incidentT.SetStyles(ts)
|
incidentT.SetStyles(ts)
|
||||||
|
|
||||||
alertT := table.New(table.WithFocused(true))
|
alertT := table.New(table.WithFocused(true), table.WithKeyMap(tableKeyMap("f")))
|
||||||
alertT.SetStyles(ts)
|
alertT.SetStyles(ts)
|
||||||
|
|
||||||
archivedT := table.New(table.WithFocused(true))
|
archivedT := table.New(table.WithFocused(true), table.WithKeyMap(tableKeyMap()))
|
||||||
archivedT.SetStyles(ts)
|
archivedT.SetStyles(ts)
|
||||||
|
|
||||||
schedT := table.New(table.WithFocused(true))
|
schedT := table.New(table.WithFocused(true), table.WithKeyMap(tableKeyMap("d")))
|
||||||
schedT.SetStyles(ts)
|
schedT.SetStyles(ts)
|
||||||
|
|
||||||
pickerT := table.New(table.WithFocused(true))
|
pickerT := table.New(table.WithFocused(true), table.WithKeyMap(tableKeyMap()))
|
||||||
pickerT.SetStyles(ts)
|
pickerT.SetStyles(ts)
|
||||||
|
|
||||||
manageT := table.New(table.WithFocused(true))
|
manageT := table.New(table.WithFocused(true), table.WithKeyMap(tableKeyMap("d", "k")))
|
||||||
manageT.SetStyles(ts)
|
manageT.SetStyles(ts)
|
||||||
|
|
||||||
// Sized by the first tea.WindowSizeMsg; built here so it carries the default
|
// Sized by the first tea.WindowSizeMsg; built here so it carries the default
|
||||||
@@ -356,6 +362,31 @@ func (m Model) Init() tea.Cmd {
|
|||||||
|
|
||||||
// ── Table rebuilders ───────────────────────────────────────────────────────
|
// ── Table rebuilders ───────────────────────────────────────────────────────
|
||||||
|
|
||||||
|
// tableKeyMap is the bubbles table keymap without the given keys.
|
||||||
|
//
|
||||||
|
// The table's defaults claim several letters -- k up, d half a page down, f a
|
||||||
|
// page down -- and the dashboard hands every key to the table before the
|
||||||
|
// section's own handler reads the cursor. A letter that is both, like k for
|
||||||
|
// API keys in Users, therefore moved the cursor and then acted on the row it
|
||||||
|
// had moved to. Each table gives up the letters its section acts on; the
|
||||||
|
// arrow keys and the rest of the defaults are untouched.
|
||||||
|
func tableKeyMap(reserved ...string) table.KeyMap {
|
||||||
|
km := table.DefaultKeyMap()
|
||||||
|
for _, b := range []*key.Binding{
|
||||||
|
&km.LineUp, &km.LineDown, &km.PageUp, &km.PageDown,
|
||||||
|
&km.HalfPageUp, &km.HalfPageDown, &km.GotoTop, &km.GotoBottom,
|
||||||
|
} {
|
||||||
|
var keep []string
|
||||||
|
for _, k := range b.Keys() {
|
||||||
|
if !slices.Contains(reserved, k) {
|
||||||
|
keep = append(keep, k)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
b.SetKeys(keep...)
|
||||||
|
}
|
||||||
|
return km
|
||||||
|
}
|
||||||
|
|
||||||
// setRows replaces a table's rows and keeps its cursor in a state the rest of
|
// setRows replaces a table's rows and keeps its cursor in a state the rest of
|
||||||
// this package can rely on: valid whenever the table has any rows at all.
|
// this package can rely on: valid whenever the table has any rows at all.
|
||||||
//
|
//
|
||||||
|
|||||||
@@ -0,0 +1,84 @@
|
|||||||
|
package tui
|
||||||
|
|
||||||
|
import (
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"git.ryuvia.com/niklas/terdut-tui/internal/api"
|
||||||
|
tea "github.com/charmbracelet/bubbletea"
|
||||||
|
)
|
||||||
|
|
||||||
|
// threeUsers is the Users section with the cursor on the last of three users.
|
||||||
|
func threeUsers() Model {
|
||||||
|
m := onUsers([]api.User{
|
||||||
|
{ID: 1, Username: "niklas"},
|
||||||
|
{ID: 2, Username: "anna"},
|
||||||
|
{ID: 3, Username: "erik"},
|
||||||
|
})
|
||||||
|
m.userManageTable.SetCursor(2)
|
||||||
|
return m
|
||||||
|
}
|
||||||
|
|
||||||
|
// The table used to see k before the section did, take it as "up", and the
|
||||||
|
// handler then opened API keys for the user above the one selected.
|
||||||
|
func TestUsers_APIKeysOpenForTheSelectedUser(t *testing.T) {
|
||||||
|
m, _ := press(t, threeUsers(), "k")
|
||||||
|
if m.mode != modeAPIKeyMenu {
|
||||||
|
t.Fatalf("expected the API key menu, got mode %v", m.mode)
|
||||||
|
}
|
||||||
|
if m.selectedUser.Username != "erik" {
|
||||||
|
t.Errorf("API keys opened for %s, want erik", m.selectedUser.Username)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Same collision with d, which the table read as half a page down: the delete
|
||||||
|
// confirmation named a different user than the one under the cursor.
|
||||||
|
func TestUsers_DeleteTargetsTheSelectedUser(t *testing.T) {
|
||||||
|
m := threeUsers()
|
||||||
|
m.userManageTable.SetCursor(0)
|
||||||
|
m, _ = press(t, m, "d")
|
||||||
|
if m.mode != modeConfirm || m.selectedUser.Username != "niklas" {
|
||||||
|
t.Errorf("delete asked about %q in mode %v, want niklas", m.selectedUser.Username, m.mode)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestSchedule_DeleteTargetsTheSelectedDay(t *testing.T) {
|
||||||
|
m := sized()
|
||||||
|
m.activeSection = sectionSchedule
|
||||||
|
m.scheduleEntries = []api.ScheduleEntry{
|
||||||
|
{ID: 10, UserID: 1, Username: "niklas", Date: m.scheduleWindow.Format("2006-01-02")},
|
||||||
|
{ID: 11, UserID: 2, Username: "anna", Date: m.scheduleWindow.AddDate(0, 0, 1).Format("2006-01-02")},
|
||||||
|
}
|
||||||
|
m.scheduleDays = buildScheduleDays(m.scheduleWindow, m.scheduleEntries)
|
||||||
|
m.rebuildScheduleTable()
|
||||||
|
m.scheduleTable.SetCursor(0)
|
||||||
|
m, _ = press(t, m, "d")
|
||||||
|
if m.pendingDeleteEntry == nil || m.pendingDeleteEntry.ID != 10 {
|
||||||
|
t.Errorf("schedule delete targeted %+v, want entry 10", m.pendingDeleteEntry)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// f cycles the filter; it must not also page the cursor down.
|
||||||
|
func TestFilter_DoesNotMoveTheCursor(t *testing.T) {
|
||||||
|
m := sized()
|
||||||
|
m.incidents = make([]api.Incident, 40)
|
||||||
|
for i := range m.incidents {
|
||||||
|
m.incidents[i] = api.Incident{ID: int64(i + 1), Title: "x", Status: api.StatusTriggered, TriggeredAt: time.Now()}
|
||||||
|
}
|
||||||
|
m.rebuildIncidentTable()
|
||||||
|
m, _ = press(t, m, "f")
|
||||||
|
if c := m.incidentTable.Cursor(); c != 0 {
|
||||||
|
t.Errorf("f moved the cursor to %d", c)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// The arrow keys still move the users table, now that k is an action there.
|
||||||
|
func TestUsers_ArrowKeysStillNavigate(t *testing.T) {
|
||||||
|
m := threeUsers()
|
||||||
|
next, _ := m.Update(keyUp())
|
||||||
|
if c := next.(Model).userManageTable.Cursor(); c != 1 {
|
||||||
|
t.Errorf("up arrow left the cursor on %d, want 1", c)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func keyUp() tea.KeyMsg { return tea.KeyMsg{Type: tea.KeyUp} }
|
||||||
Reference in New Issue
Block a user