diff --git a/src/ui/history_view.go b/src/ui/history_view.go index 6f93bb9..78a0b2e 100644 --- a/src/ui/history_view.go +++ b/src/ui/history_view.go @@ -104,35 +104,46 @@ func (h *historyHeader) SetText(text string) { h.label.SetText(text) } +// historyHeaders are the History table's column captions, in column order. The +// Time caption is built per update because it carries the sort direction arrow. +var historyHeaders = [...]string{"Time", "Trigger", "Job", "State", "Detail", "Log"} + func newHistoryView(events *[]event) (*fyne.Container, func()) { descending := false headerText := func(id widget.TableCellID) string { - headers := []string{"Time", "Trigger", "Job", "State", "Detail", "Log"} if id.Row < 0 && id.Col == 0 { if descending { return "Time ▼" } return "Time ▲" } - if id.Row < 0 && id.Col >= 0 && id.Col < len(headers) { - return headers[id.Col] + if id.Row < 0 && id.Col >= 0 && id.Col < len(historyHeaders) { + return historyHeaders[id.Col] } return "" } - sortedEvents := func() []event { - result := append([]event(nil), (*events)...) - sort.SliceStable(result, func(left int, right int) bool { + + // rows is the sorted snapshot every callback below reads — both the length + // callback and the cells, which must agree on the same slice. A full redraw + // issues one update call per visible cell, so sorting inside the cell + // callback re-sorted the whole event list a hundred times per Refresh. + // resort() is therefore the only place the order changes, and it runs once + // per redraw: at build time, on a sort toggle, and from refresh(). + var rows []event + resort := func() { + rows = append(rows[:0], (*events)...) + sort.SliceStable(rows, func(left int, right int) bool { if descending { - return result[left].Time > result[right].Time + return rows[left].Time > rows[right].Time } - return result[left].Time < result[right].Time + return rows[left].Time < rows[right].Time }) - return result } + resort() table := widget.NewTable( func() (int, int) { - return len(*events), 6 + return len(rows), len(historyHeaders) }, func() fyne.CanvasObject { label := widget.NewLabel("") @@ -140,10 +151,7 @@ func newHistoryView(events *[]event) (*fyne.Container, func()) { return label }, func(id widget.TableCellID, item fyne.CanvasObject) { - label := item.(*widget.Label) - label.SetText(historyCellText(id, sortedEvents())) - label.TextStyle = fyne.TextStyle{} - label.Refresh() + item.(*widget.Label).SetText(historyCellText(id, rows)) }, ) table.ShowHeaderRow = true @@ -156,6 +164,7 @@ func newHistoryView(events *[]event) (*fyne.Container, func()) { if id.Row < 0 && id.Col == 0 { h.OnTapped = func() { descending = !descending + resort() table.Refresh() } } else { @@ -173,10 +182,12 @@ func newHistoryView(events *[]event) (*fyne.Container, func()) { table.SetColumnWidth(4, 260) table.SetColumnWidth(5, logColumnWidth(*events)) - // refresh recomputes the content-fit Log column width before redrawing, so - // newly recorded events with longer file names widen the column instead of - // being truncated. + // refresh re-reads the event list into the sorted snapshot and recomputes + // the content-fit Log column width before redrawing, so newly recorded + // events appear in the current sort order and longer file names widen the + // column instead of being truncated. refresh := func() { + resort() table.SetColumnWidth(5, logColumnWidth(*events)) table.Refresh() } diff --git a/src/ui/history_view_test.go b/src/ui/history_view_test.go index 55e6f18..debd488 100644 --- a/src/ui/history_view_test.go +++ b/src/ui/history_view_test.go @@ -6,6 +6,8 @@ import ( "gitea.mixdep.ru/mix/gosentry/src/domain" + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/test" "fyne.io/fyne/v2/widget" ) @@ -127,6 +129,95 @@ func TestLogFileName(t *testing.T) { } } +// TestHistorySortToggleKeepsRowsInSync is the regression guard for F11: the +// table now reads one cached sorted snapshot instead of re-sorting inside every +// cell callback, so the length callback and the cells have to be refilled +// together. If either the sort toggle or refresh stops calling resort(), the +// row count and the cell contents disagree — which no compiler check catches. +func TestHistorySortToggleKeepsRowsInSync(t *testing.T) { + testApp := test.NewApp() + defer testApp.Quit() + + events := []event{ + {Time: "2026-06-01 10:00:00", JobName: "A"}, + {Time: "2026-06-01 11:00:00", JobName: "B"}, + {Time: "2026-06-01 12:00:00", JobName: "C"}, + } + content, refresh := newHistoryView(&events) + table, ok := content.Objects[0].(*widget.Table) + if !ok { + t.Fatal("history view does not wrap a table") + } + + rowCount := func() int { + t.Helper() + rows, cols := table.Length() + if cols != len(historyHeaders) { + t.Errorf("column count = %d, want %d", cols, len(historyHeaders)) + } + return rows + } + // Column 2 is the Job name, the field these fixtures vary. + jobAt := func(row int) string { + t.Helper() + cell := table.CreateCell() + table.UpdateCell(widget.TableCellID{Row: row, Col: 2}, cell) + return cell.(*widget.Label).Text + } + // The sort toggle lives on the Time header cell, which is only wired up + // when UpdateHeader runs for it. + header := table.CreateHeader() + table.UpdateHeader(widget.TableCellID{Row: -1, Col: 0}, header) + timeHeader, ok := header.(*historyHeader) + if !ok { + t.Fatal("history table header is not a historyHeader") + } + assertOrder := func(when string, want ...string) { + t.Helper() + if got := rowCount(); got != len(want) { + t.Fatalf("%s: row count = %d, want %d", when, got, len(want)) + } + for row, name := range want { + if got := jobAt(row); got != name { + t.Errorf("%s: row %d = %q, want %q", when, row, got, name) + } + } + } + + assertOrder("ascending", "A", "B", "C") + + test.Tap(timeHeader) + assertOrder("descending", "C", "B", "A") + + // A new run arrives while the table is sorted newest-first: it must be + // counted and placed in the order currently on screen, not the build-time one. + events = append(events, event{Time: "2026-06-01 13:00:00", JobName: "D"}) + refresh() + assertOrder("descending after refresh", "D", "C", "B", "A") + + test.Tap(timeHeader) + assertOrder("ascending after refresh", "A", "B", "C", "D") +} + +// TestHistoryCellTemplateIsPlainText guards the dropped per-cell TextStyle +// assignment: the template must already carry the zero style, since nothing +// resets it any more. +func TestHistoryCellTemplateIsPlainText(t *testing.T) { + testApp := test.NewApp() + defer testApp.Quit() + + var events []event + content, _ := newHistoryView(&events) + table := content.Objects[0].(*widget.Table) + label, ok := table.CreateCell().(*widget.Label) + if !ok { + t.Fatal("history cell template is not a label") + } + if label.TextStyle != (fyne.TextStyle{}) { + t.Errorf("cell template TextStyle = %+v, want the zero value", label.TextStyle) + } +} + func TestNewEventUsesConsistentTimestampShape(t *testing.T) { ev := newEvent(1, "Job", "OK", "detail") if _, err := time.Parse("2006-01-02 15:04:05", ev.Time); err != nil { diff --git a/src/ui/jobs_view.go b/src/ui/jobs_view.go index 5645e83..c751b21 100644 --- a/src/ui/jobs_view.go +++ b/src/ui/jobs_view.go @@ -166,12 +166,14 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { } selectedFolder = value filteredJobs = filteredJobIndexes(jobs, selectedFolder) - list.Refresh() if len(filteredJobs) == 0 { // The "No folder" filter is intentionally allowed to be empty. It is a // real filter choice, not an error state, so the selection is cleared. + // This path returns without reaching refreshView(), so it is the one + // place the list has to be redrawn by hand. selected = -1 updateDetails(-1) + list.Refresh() return } selected = filteredJobs[0] @@ -223,7 +225,6 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { } selected = indexOfID(jobs, created.ID) filteredJobs = filteredJobIndexes(jobs, selectedFolder) - list.Refresh() list.Select(app.DisplayIndex(filteredJobs, selected)) refreshView() }) @@ -241,7 +242,6 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { syncFromService() folderSelect.Options = folderOptions(jobs) folderSelect.Refresh() - list.Refresh() refreshView() }) }) @@ -255,7 +255,6 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { dialog.ShowError(err, w) return } - list.Refresh() refreshView() }) @@ -287,7 +286,6 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { stopAllButton.SetText("Disable auto") stopAllButton.SetIcon(theme.MediaPauseIcon()) } - list.Refresh() refreshView() } pauseButton := widget.NewButtonWithIcon("Pause", theme.MediaPauseIcon(), func() { @@ -299,8 +297,6 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { dialog.ShowError(err, w) return } - syncFromService() - list.Refresh() refreshView() }) deleteButton := widget.NewButtonWithIcon("Delete", theme.DeleteIcon(), func() { @@ -332,7 +328,6 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { } else { selected = filteredJobs[0] } - list.Refresh() if selected >= 0 { list.Select(app.DisplayIndex(filteredJobs, selected)) } diff --git a/src/ui/jobs_view_test.go b/src/ui/jobs_view_test.go index 47fee2d..9a2307f 100644 --- a/src/ui/jobs_view_test.go +++ b/src/ui/jobs_view_test.go @@ -180,6 +180,58 @@ func jobsToolbar(t *testing.T, content fyne.CanvasObject) fyne.CanvasObject { return found } +// jobsToolbarButton returns the toolbar button with the given caption. +func jobsToolbarButton(t *testing.T, content fyne.CanvasObject, text string) *widget.Button { + t.Helper() + found := findFirst(jobsToolbar(t, content), func(o fyne.CanvasObject) bool { + button, ok := o.(*widget.Button) + return ok && button.Text == text + }) + if found == nil { + t.Fatalf("jobs toolbar has no %q button", text) + } + return found.(*widget.Button) +} + +// jobsDetails narrows the search to the right pane. NewBorder keeps the centre +// object first, so panel.Objects[0] is the details pane (see jobsSidebar). +func jobsDetails(t *testing.T, content fyne.CanvasObject) fyne.CanvasObject { + t.Helper() + panel, ok := content.(*fyne.Container) + if !ok || len(panel.Objects) == 0 { + t.Fatal("jobs view is not the expected Border container") + } + return panel.Objects[0] +} + +// jobsDetailsActivity returns the "Selected job activity" list, the only +// widget.List in the details pane. +func jobsDetailsActivity(t *testing.T, content fyne.CanvasObject) *widget.List { + t.Helper() + found := findFirst(jobsDetails(t, content), func(o fyne.CanvasObject) bool { + _, ok := o.(*widget.List) + return ok + }) + if found == nil { + t.Fatal("details pane has no activity list") + } + return found.(*widget.List) +} + +// jobsDetailsTitle reads the details pane's heading, which detailsPanel builds +// as the first bold label in the pane. +func jobsDetailsTitle(t *testing.T, content fyne.CanvasObject) string { + t.Helper() + found := findFirst(jobsDetails(t, content), func(o fyne.CanvasObject) bool { + label, ok := o.(*widget.Label) + return ok && label.TextStyle.Bold + }) + if found == nil { + t.Fatal("details pane has no title label") + } + return found.(*widget.Label).Text +} + func jobsViewToggle(t *testing.T, content fyne.CanvasObject) *widget.Button { t.Helper() found := findFirst(jobsSidebar(t, content), func(o fyne.CanvasObject) bool { @@ -295,6 +347,71 @@ func TestJobsSidebarWidthIsItsContent(t *testing.T) { } } +// TestToolbarButtonRedrawsRowAndDetails is the regression guard for F12: the +// toolbar handlers no longer re-read the service or refresh the list +// themselves, so refreshView alone has to re-snapshot the jobs and repopulate +// the details pane. If it ever stops doing either, the row renders a stale +// status and the details lose the selection — neither is a compile error. +func TestToolbarButtonRedrawsRowAndDetails(t *testing.T) { + testApp := test.NewApp() + defer testApp.Quit() + w := testApp.NewWindow("test") + defer w.Close() + + store := newTestStore(t) + jobs := []domain.Job{ + {ID: 1, Name: "First", Schedule: "@every 1m", Command: "echo one", Enabled: true}, + {ID: 2, Name: "Second", Schedule: "@every 2m", Command: "echo two", Enabled: true}, + } + svc := app.NewService(store, jobs) + defer svc.Stop() + + content, _ := newJobsView(w, svc) + w.SetContent(content) + + list := jobsList(t, content) + // Row layout: VBox(nameLine, meta, status), nameLine = Border(name, inlineStatus). + rowText := func(id int) (name string, status string) { + t.Helper() + row := list.CreateItem().(*fyne.Container) + list.UpdateItem(id, row) + nameLine := row.Objects[0].(*fyne.Container) + return nameLine.Objects[0].(*widget.Label).Text, row.Objects[2].(*widget.Label).Text + } + + activity := jobsDetailsActivity(t, content) + + list.Select(1) + if got := jobsDetailsTitle(t, content); got != "Second" { + t.Fatalf("details title after selecting row 1 = %q, want %q", got, "Second") + } + if _, status := rowText(1); status == "Paused" { + t.Fatal("the second job should start enabled") + } + if got := activity.Length(); got != 0 { + t.Fatalf("activity rows before the tap = %d, want 0", got) + } + + test.Tap(jobsToolbarButton(t, content, "Pause")) + + if svc.Jobs()[1].Enabled { + t.Fatal("tapping Pause did not reach the service") + } + name, status := rowText(1) + if name != "Second" || status != "Paused" { + t.Errorf("row 1 after Pause = (%q, %q), want (%q, %q)", name, status, "Second", "Paused") + } + if got := jobsDetailsTitle(t, content); got != "Second" { + t.Errorf("details title after Pause = %q, want the selection kept at %q", got, "Second") + } + // The pause writes an activity record. Seeing it here is what proves + // refreshView repopulated the details pane rather than leaving the panel on + // the snapshot it held before the tap. + if got := activity.Length(); got != 1 { + t.Errorf("activity rows after the tap = %d, want the pause record", got) + } +} + func TestViewToggleTextNamesTheAction(t *testing.T) { cases := []struct { current domain.JobListView