From 60aceb75afd67ad248f0da710e571921e6c0ff66 Mon Sep 17 00:00:00 2001 From: mixeme Date: Mon, 27 Jul 2026 13:47:39 +0300 Subject: [PATCH] fix: window minimum, stock spacing layout, sidebar width floor Part A stages 1-3 of the GUI layout cleanup plan: - Stage 1 (F1-F3): delete settingsControlWidth's redundant wrapper (the Border centre slot already stretches controls), truncate the config path label, and name the default window size so it can be asserted against. Settings no longer widens the window past what it asks for. - Stage 2 (F4-F5): drop compactVBoxLayout for the stock layout.NewCustomPaddedVBoxLayout and a single derived rowOverlap() spacing, replacing three hand-tuned spacing constants. - Stage 3 (F7): delete the inert 400px sidebar width floor; the Border left slot already renders it at content MinSize. See docs/PLAN-gui-layout.md. --- src/ui/jobs_view.go | 21 +++---------- src/ui/jobs_view_details.go | 3 +- src/ui/jobs_view_test.go | 61 +++++++++++++++++++++++++++++------- src/ui/layout.go | 48 +++++----------------------- src/ui/layout_test.go | 28 +++++++++++++++++ src/ui/mainwindow_test.go | 24 ++++++++++++++ src/ui/run.go | 11 +++++-- src/ui/settings_view.go | 31 +++++++++--------- src/ui/settings_view_test.go | 24 ++++++++++++++ 9 files changed, 162 insertions(+), 89 deletions(-) create mode 100644 src/ui/layout_test.go diff --git a/src/ui/jobs_view.go b/src/ui/jobs_view.go index 62b4b93..5645e83 100644 --- a/src/ui/jobs_view.go +++ b/src/ui/jobs_view.go @@ -16,24 +16,12 @@ import ( const allFolders = "All" const noFolder = "No folder" -const minJobsSidebarWidth float32 = 400 // maxJobActivityRows caps the "Selected job activity" panel to the most recent // entries. The full per-job history (up to maxJobLogs) remains in the History // view; this panel is a quick at-a-glance summary anchored below the output. const maxJobActivityRows = 3 -// detailRowSpacing is the (negative) gap applied between metadata rows in the -// details panel. Pulling rows together overlaps the labels' built-in vertical -// padding, tightening the block so it fits comfortably on 720p screens. -const detailRowSpacing float32 = -8 - -// jobRowSpacing is the (negative) gap between the name, metadata, and status -// lines within each job list row. Like the details panel, it overlaps the -// labels' built-in vertical padding so each row reads as one compact block and -// more jobs are visible without scrolling. -const jobRowSpacing float32 = -8 - // newJobsView builds the Jobs tab: list sidebar, details panel, and toolbar. // It returns the assembled panel and a refresh function the caller invokes // whenever the service state may have changed (e.g., from the event subscriber @@ -109,8 +97,8 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { // applyRowMode expresses the current view mode as visibility on the row's // four labels. widget.List caches the row template's MinSize, and // list.Refresh() re-creates the template and recomputes it, so hiding lines - // is what actually shrinks the rows: compactVBoxLayout and the border layout - // both skip hidden children when measuring. + // is what actually shrinks the rows: layout.NewCustomPaddedVBoxLayout and the + // border layout both skip hidden children when measuring. applyRowMode := func(inlineStatus, meta, status fyne.CanvasObject) { if listView.IsCompact() { inlineStatus.Show() @@ -136,7 +124,7 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { status := widget.NewLabel("status") applyRowMode(inlineStatus, meta, status) nameLine := container.NewBorder(nil, nil, nil, inlineStatus, name) - return container.New(compactVBoxLayout{spacing: jobRowSpacing}, nameLine, meta, status) + return container.New(layout.NewCustomPaddedVBoxLayout(rowOverlap()), nameLine, meta, status) }, func(id widget.ListItemID, item fyne.CanvasObject) { row := item.(*fyne.Container) @@ -362,7 +350,6 @@ func newJobsView(w fyne.Window, svc *app.Service) (fyne.CanvasObject, func()) { sidebarHeader := container.NewVBox(globalControls, widget.NewSeparator(), filterRow, toolbar) sidebar := container.NewBorder(sidebarHeader, nil, nil, nil, list) - fixedSidebar := container.New(minWidthLayout{width: minJobsSidebarWidth}, sidebar) - panel := container.NewBorder(nil, nil, fixedSidebar, nil, container.NewPadded(dp.container())) + panel := container.NewBorder(nil, nil, sidebar, nil, container.NewPadded(dp.container())) return panel, refreshView } diff --git a/src/ui/jobs_view_details.go b/src/ui/jobs_view_details.go index 9373eac..6b5b70a 100644 --- a/src/ui/jobs_view_details.go +++ b/src/ui/jobs_view_details.go @@ -6,6 +6,7 @@ import ( "fyne.io/fyne/v2" "fyne.io/fyne/v2/container" + "fyne.io/fyne/v2/layout" "fyne.io/fyne/v2/theme" "fyne.io/fyne/v2/widget" ) @@ -121,7 +122,7 @@ func (d *detailsPanel) container() fyne.CanvasObject { // keeping the details pane usable on 720p screens where a single column of // ten rows pushes the minimum window height past the available space. capW := detailCaptionWidth() - rows := container.New(compactVBoxLayout{spacing: detailRowSpacing}, + rows := container.New(layout.NewCustomPaddedVBoxLayout(rowOverlap()), detailRowPair(capW, "Folder", d.folder, "Schedule", d.schedule), detailRowPair(capW, "Command", d.command, "Arguments", d.arguments), detailRowPair(capW, "Run mode", d.runMode, "Overlap policy", d.overlapPolicy), diff --git a/src/ui/jobs_view_test.go b/src/ui/jobs_view_test.go index cf89a4e..47fee2d 100644 --- a/src/ui/jobs_view_test.go +++ b/src/ui/jobs_view_test.go @@ -137,21 +137,17 @@ func findFirst(root fyne.CanvasObject, match func(fyne.CanvasObject) bool) fyne. // jobsSidebar narrows the search to the left pane. The details panel has a // widget.List of its own (the activity log), so a search from the whole view -// would find the wrong one. +// would find the wrong one. newJobsView assembles the panel as +// container.NewBorder(nil, nil, sidebar, nil, ...); NewBorder keeps the centre +// object first and appends the border slots after it, so panel.Objects[1] is +// the left (sidebar) slot. func jobsSidebar(t *testing.T, content fyne.CanvasObject) fyne.CanvasObject { t.Helper() - found := findFirst(content, func(o fyne.CanvasObject) bool { - wrapper, ok := o.(*fyne.Container) - if !ok { - return false - } - _, ok = wrapper.Layout.(minWidthLayout) - return ok - }) - if found == nil { - t.Fatal("jobs view has no fixed-width sidebar") + panel, ok := content.(*fyne.Container) + if !ok || len(panel.Objects) < 2 { + t.Fatal("jobs view is not the expected Border container") } - return found + return panel.Objects[1] } func jobsList(t *testing.T, content fyne.CanvasObject) *widget.List { @@ -166,6 +162,24 @@ func jobsList(t *testing.T, content fyne.CanvasObject) *widget.List { return found.(*widget.List) } +// jobsToolbar finds the add/edit/run/pause/delete button row inside the +// sidebar, identified by its first child being the "New job" button. +func jobsToolbar(t *testing.T, content fyne.CanvasObject) fyne.CanvasObject { + t.Helper() + found := findFirst(jobsSidebar(t, content), func(o fyne.CanvasObject) bool { + wrapper, ok := o.(*fyne.Container) + if !ok || len(wrapper.Objects) == 0 { + return false + } + button, ok := wrapper.Objects[0].(*widget.Button) + return ok && button.Text == "New job" + }) + if found == nil { + t.Fatal("jobs sidebar has no toolbar row") + } + return found +} + func jobsViewToggle(t *testing.T, content fyne.CanvasObject) *widget.Button { t.Helper() found := findFirst(jobsSidebar(t, content), func(o fyne.CanvasObject) bool { @@ -258,6 +272,29 @@ func TestJobListViewCompactConfigOpensCompact(t *testing.T) { } } +// TestJobsSidebarWidthIsItsContent is the regression guard for F7: nothing +// but the sidebar's own content (here, the toolbar row) should impose a +// width floor on it. +func TestJobsSidebarWidthIsItsContent(t *testing.T) { + testApp := test.NewApp() + defer testApp.Quit() + w := testApp.NewWindow("test") + defer w.Close() + + store := newTestStore(t) + svc := app.NewService(store, nil) + defer svc.Stop() + + content, _ := newJobsView(w, svc) + w.SetContent(content) + + sidebarWidth := jobsSidebar(t, content).MinSize().Width + toolbarWidth := jobsToolbar(t, content).MinSize().Width + if sidebarWidth != toolbarWidth { + t.Errorf("sidebar MinSize().Width = %v, want it to equal the toolbar row's %v", sidebarWidth, toolbarWidth) + } +} + func TestViewToggleTextNamesTheAction(t *testing.T) { cases := []struct { current domain.JobListView diff --git a/src/ui/layout.go b/src/ui/layout.go index 805c79f..375b962 100644 --- a/src/ui/layout.go +++ b/src/ui/layout.go @@ -37,47 +37,13 @@ func (l minWidthLayout) Layout(objects []fyne.CanvasObject, size fyne.Size) { } } -// compactVBoxLayout stacks children vertically with a configurable gap between -// them, producing tighter rows than container.NewVBox (which inserts -// theme.Padding() between every child). A negative spacing pulls neighbouring -// rows together so they overlap the labels' built-in vertical padding, which is -// how the details metadata is condensed to fit 720p screens. -type compactVBoxLayout struct { - spacing float32 -} - -func (l compactVBoxLayout) MinSize(objects []fyne.CanvasObject) fyne.Size { - var w, h float32 - var visible int - for _, o := range objects { - if !o.Visible() { - continue - } - min := o.MinSize() - if min.Width > w { - w = min.Width - } - h += min.Height - visible++ - } - if visible > 1 { - h += l.spacing * float32(visible-1) - } - return fyne.NewSize(w, h) -} - -func (l compactVBoxLayout) Layout(objects []fyne.CanvasObject, size fyne.Size) { - var y float32 - for _, o := range objects { - if !o.Visible() { - continue - } - h := o.MinSize().Height - o.Move(fyne.NewPos(0, y)) - o.Resize(fyne.NewSize(size.Width, h)) - y += h + l.spacing - } -} +// rowOverlap is the (negative) gap that pulls stacked label rows together by +// exactly one label's vertical inner padding. Two adjacent labels each inset +// their text by theme.InnerPadding(), so the whitespace between two lines of +// text is double what a single row needs; removing one label's worth +// condenses the block without letting the text lines touch. Derived rather +// than hard-coded so it follows a theme that changes SizeNameInnerPadding. +func rowOverlap() float32 { return -theme.InnerPadding() } // fixedHeightLayout forces its contents to a fixed height while leaving the // width to the parent container. It is used to reserve a stable amount of space diff --git a/src/ui/layout_test.go b/src/ui/layout_test.go new file mode 100644 index 0000000..b6929ee --- /dev/null +++ b/src/ui/layout_test.go @@ -0,0 +1,28 @@ +package ui + +import ( + "testing" + + "fyne.io/fyne/v2/test" + "fyne.io/fyne/v2/theme" +) + +// TestRowOverlapMatchesInnerPadding pins rowOverlap to theme.InnerPadding, the +// property that lets it follow a theme with a different SizeNameInnerPadding +// instead of drifting from a hand-tuned literal. +func TestRowOverlapMatchesInnerPadding(t *testing.T) { + testApp := test.NewApp() + defer testApp.Quit() + + if got, want := rowOverlap(), -theme.InnerPadding(); got != want { + t.Errorf("rowOverlap() = %v, want %v", got, want) + } + if rowOverlap() >= 0 { + t.Errorf("rowOverlap() = %v, want a negative value", rowOverlap()) + } + + testApp.Settings().SetTheme(test.NewTheme()) + if got, want := rowOverlap(), -theme.InnerPadding(); got != want { + t.Errorf("under a different theme, rowOverlap() = %v, want %v", got, want) + } +} diff --git a/src/ui/mainwindow_test.go b/src/ui/mainwindow_test.go index 01806e6..9f5f567 100644 --- a/src/ui/mainwindow_test.go +++ b/src/ui/mainwindow_test.go @@ -45,6 +45,30 @@ func newTestService(t *testing.T) *app.Service { return app.NewService(newTestStore(t), nil) } +// TestMainViewFitsTheDefaultWindowSize is the regression guard for F1: the +// assembled content must fit within the window size the app asks for, so Fyne +// never silently widens the window past it. The store's ConfigPath is +// deliberately long so the test also covers F3 — the config path label must +// not grow the Settings tab's minimum width with it. +func TestMainViewFitsTheDefaultWindowSize(t *testing.T) { + testApp := test.NewApp() + defer testApp.Quit() + + w := testApp.NewWindow("test") + defer w.Close() + + store := newTestStore(t) + store.Paths.ConfigPath = filepath.Join(t.TempDir(), "a-deliberately-long-directory-name-to-stress-the-config-path-label", "gosentry.json") + svc := app.NewService(store, nil) + defer svc.Stop() + + content, _ := newMainView(w, svc) + min := content.MinSize() + if min.Width > defaultWindowWidth || min.Height > defaultWindowHeight { + t.Errorf("content.MinSize() = %v, want within %vx%v", min, defaultWindowWidth, defaultWindowHeight) + } +} + func TestMainViewBuilds(t *testing.T) { testApp := test.NewApp() defer testApp.Quit() diff --git a/src/ui/run.go b/src/ui/run.go index 40c36ec..477d47d 100644 --- a/src/ui/run.go +++ b/src/ui/run.go @@ -15,6 +15,13 @@ import ( const appID = "ru.mixeme.gosentry.desktop" +// defaultWindowWidth and defaultWindowHeight are the size the window opens at +// on first launch (later launches restore the last size from preferences). +// Fyne enforces the assembled content's MinSize as a hard floor over these, so +// they only take effect if the content actually fits within them. +const defaultWindowWidth = 1024 +const defaultWindowHeight = 660 + // Run is the application entry point. It owns the process lifecycle — single // instance arbitration, Fyne app + window construction, tray wiring, and the // startup-timing record — and delegates all view construction to newMainView in @@ -51,8 +58,8 @@ func Run(startInTray bool) { w := a.NewWindow("GoSentry " + app.Version) configureSystemTray(a, w) prefs := a.Preferences() - winW := float32(prefs.FloatWithFallback("window.width", 1024)) - winH := float32(prefs.FloatWithFallback("window.height", 660)) + winW := float32(prefs.FloatWithFallback("window.width", defaultWindowWidth)) + winH := float32(prefs.FloatWithFallback("window.height", defaultWindowHeight)) w.Resize(fyne.NewSize(winW, winH)) svc, err := app.Open() if err != nil { diff --git a/src/ui/settings_view.go b/src/ui/settings_view.go index af88f36..dadd41e 100644 --- a/src/ui/settings_view.go +++ b/src/ui/settings_view.go @@ -18,6 +18,7 @@ import ( "fyne.io/fyne/v2/canvas" "fyne.io/fyne/v2/container" "fyne.io/fyne/v2/dialog" + "fyne.io/fyne/v2/layout" fynestorage "fyne.io/fyne/v2/storage" "fyne.io/fyne/v2/theme" "fyne.io/fyne/v2/widget" @@ -27,14 +28,8 @@ import ( // overlap policy") in full; the captions truncate, so a narrower width would // clip it. All rows share this width so their value controls stay aligned. const settingsLabelWidth float32 = 180 -const settingsControlWidth float32 = 330 const projectRepositoryURL = "https://gitea.mixdep.ru/mix/gosentry" -// settingsRowSpacing is the (negative) gap between rows of the settings form, -// overlapping each control's built-in vertical padding so the column is tighter -// and more compact, matching the condensed job details panel. -const settingsRowSpacing float32 = -6 - func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { store := svc.Store() // updateSaveState compares the form to the saved config and enables Save only @@ -245,13 +240,13 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { // the read-only About block. Save spans the full width below both columns. leftColumn := container.NewVBox( settingsSection("Application", - settingsRow("Autostart", container.New(minWidthLayout{width: settingsControlWidth}, startOnLogin)), + settingsRow("Autostart", startOnLogin), // Autostart status sits on its own row, aligned under the checkbox via an // empty caption, so the Application section fits in a half-width column. settingsRow("", autostartStatus), - settingsRow("Tray", container.New(minWidthLayout{width: settingsControlWidth}, minimizeToTray)), - settingsRow("Notifications", container.New(minWidthLayout{width: settingsControlWidth}, notifications)), - settingsRow("Theme", container.New(minWidthLayout{width: settingsControlWidth}, themeSelect)), + settingsRow("Tray", minimizeToTray), + settingsRow("Notifications", notifications), + settingsRow("Theme", themeSelect), ), widget.NewSeparator(), // Queue holds the execution mode and overlap policy comboboxes. Like @@ -259,18 +254,22 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { // layout) so the comboboxes keep a visible gap between them. container.NewVBox( widget.NewLabelWithStyle("Queue", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}), - settingsRow("Execution mode", container.New(minWidthLayout{width: settingsControlWidth}, executionModeSelect)), - settingsRow("Default overlap policy", container.New(minWidthLayout{width: settingsControlWidth}, overlapPolicySelect)), - settingsRow("Default timeout (s)", container.New(minWidthLayout{width: settingsControlWidth}, defaultTimeout)), + settingsRow("Execution mode", executionModeSelect), + settingsRow("Default overlap policy", overlapPolicySelect), + settingsRow("Default timeout (s)", defaultTimeout), ), ) + // Truncating keeps a long config path from forcing the Settings tab's + // minimum width to track the path length instead of the layout itself. + configPathLabel := widget.NewLabel(store.Paths.ConfigPath) + configPathLabel.Truncation = fyne.TextTruncateClip rightColumn := container.NewVBox( // Storage holds editable entry fields. It uses the default VBox spacing // (not the condensed section layout) so the entry boxes keep a visible // gap between them instead of merging into one block. container.NewVBox( widget.NewLabelWithStyle("Storage", fyne.TextAlignLeading, fyne.TextStyle{Bold: true}), - settingsRow("Config JSON", widget.NewLabel(store.Paths.ConfigPath)), + settingsRow("Config JSON", configPathLabel), settingsRow("Jobs file", container.NewBorder(nil, nil, nil, jobsFileBrowse, jobsFile)), // Browse stays rightmost so it lines up with the Jobs file row // above it; Open sits between it and the path it opens. @@ -313,14 +312,14 @@ func settingsView(w fyne.Window, svc *app.Service) fyne.CanvasObject { } // settingsSection groups a bold header above its rows using the tight -// settingsRowSpacing so a block of label rows reads as one compact unit. The +// rowOverlap spacing so a block of label rows reads as one compact unit. The // caller keeps separators and entry-heavy sections in the surrounding VBox so // they retain the theme's normal spacing. func settingsSection(title string, rows ...fyne.CanvasObject) fyne.CanvasObject { children := make([]fyne.CanvasObject, 0, len(rows)+1) children = append(children, widget.NewLabelWithStyle(title, fyne.TextAlignLeading, fyne.TextStyle{Bold: true})) children = append(children, rows...) - return container.New(compactVBoxLayout{spacing: settingsRowSpacing}, children...) + return container.New(layout.NewCustomPaddedVBoxLayout(rowOverlap()), children...) } func fyneVersion() string { diff --git a/src/ui/settings_view_test.go b/src/ui/settings_view_test.go index 865156d..6b37a7f 100644 --- a/src/ui/settings_view_test.go +++ b/src/ui/settings_view_test.go @@ -3,6 +3,9 @@ package ui import ( "path/filepath" "testing" + + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/widget" ) // TestSettingsFolderPath covers the path the "Open" button beside the logs @@ -33,3 +36,24 @@ func TestSettingsFolderPath(t *testing.T) { }) } } + +// TestSettingsRowStretchesItsControl is the property that makes F2's removal +// of settingsControlWidth invisible: settingsRow puts the value in a Border +// centre slot, which already stretches it to the column width on its own, so +// wrapping it in a fixed-width layout was redundant. +func TestSettingsRowStretchesItsControl(t *testing.T) { + entry := widget.NewEntry() + row := settingsRow("Label", entry) + + baseWidth := entry.Size().Width + wide := fyne.NewSize(row.MinSize().Width+200, row.MinSize().Height) + row.Resize(wide) + + // Only the caption column and one inter-column padding come out of the + // extra width; the rest must reach the control. Requiring most of the + // 200px growth to show up on the entry is what a reinstated fixed-width + // wrapper around it would break. + if got := entry.Size().Width; got < baseWidth+150 { + t.Errorf("control did not stretch to fill the row: entry width = %v, want at least %v", got, baseWidth+150) + } +}