diff --git a/CHANGELOG.md b/CHANGELOG.md index 07fca07..cf137bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,27 @@ because it turns other people's test suites red. ### Changed +- **Nine things the owner saw in the running window.** A field's + explanation opens on the same raised surface as an open list, so it no + longer lies flat on the section it covers. A list of formats keeps its + tick in front of the picture and the word, the shape it had before the + tick moved to the end of every row. A list opens downward whenever a few + rows fit under its box, shorter and scrolling, and turns upward only for + a box standing just over the bar at the foot. A menu no longer opens + marked as holding the keyboard when the window comes to the front, which + is what made the first menu on the first screen blue and every other one + grey - the mark is drawn for the keyboard alone, as it always was meant to + be. A box for a name, a template, a file name, a password or a list of + sizes is the width of two number boxes rather than of the whole row, and + only a path still takes the row. The tick in a checked box fills its + square. The first start is sized for the first screen rather than the + tallest one - see the entry on the window's height further down. Preview, + Choose, Duplicate and Add a batch have a raised + face rather than an outline round nothing. And the busy face - the frozen + form, Cancel, the bar - waits a moment before it appears, so a preview + that is over in a blink no longer flashes it and takes it back, and the + run buttons stand where they stood once the work is done. + - **The window draws its own buttons, switches and choosers.** A button now has one filled face for the action that does the work and an outline for the ones beside it, lightens under the pointer, darkens when pressed, and shows a @@ -84,13 +105,15 @@ because it turns other people's test suites red. binary is about 800 kB larger for it. The command line binary carries no font and is unchanged. -- **The window opens as tall as its screens need, and no taller than a - 1080p screen.** A first start used to open at a height measured against the - forms of an earlier version, and left a band of nothing under the form once - the forms grew shorter. It now opens exactly tall enough for the tallest - screen to show whole - today that is 917 px - and never taller than fits a - 1080p screen with its taskbar. A window you have resized still comes back at - the size you left it. +- **The window opens as tall as the screen it opens on needs, and no taller + than a 1080p screen.** A first start used to open at a height measured + against the forms of an earlier version, and left a band of nothing under + the form once the forms grew shorter. It now opens exactly tall enough for + the first screen to show whole - today that is 851 px - and never taller + than fits a 1080p screen with its taskbar. The taller screens scroll a + little on arrival, which the batch screen does from the second batch on in + any case. A window you have resized still comes back at the size you left + it. - **The files inside an archive are a table with one row of headings.** The table on the batch screen named every column again in every row, so two diff --git a/internal/guard/boxwidth_test.go b/internal/guard/boxwidth_test.go index 4567ed0..34e16d3 100644 --- a/internal/guard/boxwidth_test.go +++ b/internal/guard/boxwidth_test.go @@ -329,3 +329,103 @@ func TestADeclaredSettingStandsOnTheSameEdgeAsTheFieldsAboveIt(t *testing.T) { }) } } + +// Only a path takes the whole row. +// +// The owner's question from the running window on 2026-09-21: why are Batch +// name, File names, Password, Spread, Kind of case and Manifest file name so +// long. They took the row on the sentence that free text has no length to +// promise, which is true and beside the point - a name is a short thing, and +// a box 806 px wide for one promises something the value will never be, the +// same defect TestABoxForANumberIsNotAsWideAsTheForm holds for a number. +// The one value that can be long is a path, so the output directory keeps +// the row and everything else typed into these screens is held under half +// the column. +// +// Every box a person types into, on all three screens, rather than the six +// the owner named: the seventh is the one nobody names. The folded sections +// are opened first, because a box that is not on the screen has no laid out +// width (the lesson of the guard above), and the two boxes the report named +// inside them - a password, a kind of case - are exactly the ones a guard +// reading the open screen would never see. +func TestOnlyAPathTakesTheWholeRow(t *testing.T) { + ourTheme(t) + host := newFakeHost(t) + window.Open(host) + if host.content == nil { + t.Fatal("opening the window put no screen in it") + } + w := test.NewWindow(host.content) + t.Cleanup(w.Close) + layOut := func() { + w.Resize(fyne.NewSize(window.LargestOpening.Width, 1599)) + w.Resize(fyne.NewSize(window.LargestOpening.Width, 1600)) + } + layOut() + + batches := selectTab(t, host.content, text.TabRecipe()) + // A zip holds the one password box in the registry, and the notes hold the + // kind of case. + if picker, ok := controlUnder(batches, text.FieldFormat()).(*parts.Chooser); ok { + picker.SetSelected("zip") + } else { + t.Fatal("the first batch has no format list, so this guard read the wrong tree") + } + openFold(t, batches, "", text.SettingsFor("zip")) + openFold(t, batches, text.BatchHeading(1), text.SectionManifestNotes()) + layOut() + + half := float32(parts.ColumnWidth) / 2 + checked, paths := 0, 0 + for _, tab := range []string{text.TabOneTarget(), text.TabPresets(), text.TabRecipe()} { + screen := selectTab(t, host.content, tab) + layOut() + path := controlUnder(screen, text.FieldOutputDir()) + if path == nil { + t.Fatalf("the %s screen has no output directory, so this guard cannot tell the path from the rest", tab) + } + onThePath := map[fyne.CanvasObject]bool{} + walk(path, func(o fyne.CanvasObject) { onThePath[o] = true }) + + walk(screen, func(o fyne.CanvasObject) { + box, is := o.(*parts.Entry) + if !is || !box.Visible() || box.Size().Width == 0 { + return + } + if onThePath[box] { + paths++ + return + } + checked++ + if box.Size().Width > half { + t.Errorf("%s: the box holding %q (placeholder %q) is %.0f px of a %d px column, and only a path is allowed the row", + tab, box.Text, box.PlaceHolder, box.Size().Width, parts.ColumnWidth) + } + }) + } + // The boxes the owner named, by name, so a screen that stopped drawing + // one of them is a red guard and not a smaller count. + for _, named := range []struct{ tab, label string }{ + {text.TabOneTarget(), text.FieldTargetID()}, + {text.TabOneTarget(), text.FieldNameTemplate()}, + {text.TabPresets(), text.SettingLabel("spread")}, + {text.TabRecipe(), text.FieldManifest()}, + {text.TabRecipe(), text.FieldGroup()}, + {text.TabRecipe(), text.SettingLabel("password")}, + } { + screen := selectTab(t, host.content, named.tab) + layOut() + control := controlUnder(screen, named.label) + if control == nil { + t.Errorf("%s: no field is labelled %q, so its width cannot be measured", named.tab, named.label) + continue + } + if width := typedInWidth(control); width == 0 || width > half { + t.Errorf("%s: the box under %q is %.0f px wide (nought is a box not on the screen)", named.tab, named.label, width) + } + } + if checked < 6 || paths < 3 { + t.Fatalf("checked %d boxes and %d paths across three screens, which is not the whole window", checked, paths) + } + t.Logf("%d boxes held under half the column, %d paths allowed the row", checked, paths) +} diff --git a/internal/guard/busyface_test.go b/internal/guard/busyface_test.go new file mode 100644 index 0000000..bee9769 --- /dev/null +++ b/internal/guard/busyface_test.go @@ -0,0 +1,245 @@ +package guard + +import ( + "testing" + + "fyne.io/fyne/v2" + "fyne.io/fyne/v2/test" + + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/window" +) + +// The busy face waits for work that lasts, and never arrives for work that +// does not. +// +// Reported by the owner from the running window on 2026-09-21: the whole +// window shook when Preview was pressed. Measured there: a preview of one +// file is done in about 50 ms, and in that time every box on the form was +// frozen - the toolkit draws a frozen box with a bright edge and grey words - +// a Cancel button came into the row and pushed the other two aside, a bar +// stood at nought, and then all of it was taken back. A flash, not a state. +// +// So the STATE is immediate - a second press of Generate inside the first +// moment of the first is still refused - and the FACE follows after +// BusyFaceAfter if the work is still going. Asked through the clock the host +// hands the window, held here so the moment before the face can be read: +// under the test driver a real timer would be a second writer to the widgets +// this guard is reading. Three things are held, and each is a way the delay +// could be wrong on its own: that the face is not on before the clock fires, +// that it IS on after, and that the clock was asked for the delay this +// package names rather than for nought. +func TestTheBusyFaceWaitsForWorkThatLasts(t *testing.T) { + host, content, hold := heldScreen(t) + clock := host.holdTheClock() + w := test.NewWindow(host.content) + t.Cleanup(w.Close) + w.Resize(window.LargestOpening) + + fill(t, content, text.FieldOutputDir(), t.TempDir()) + fill(t, content, text.FieldCount(), "20000") + box := entryUnder(t, content, text.FieldSize()) + if box == nil { + t.Fatal("there is no size box, so this guard read the wrong tree") + } + press(t, content, text.ButtonPreview()) + + // The moment after the press: nothing on the screen has changed yet. + if box.Disabled() { + t.Error("the size box froze the moment Preview was pressed - the flash the owner reported, for work that may be over in 50 ms") + } + if cancel := buttonNamed(content, text.ButtonCancel()); cancel != nil && cancel.Visible() { + t.Error("Cancel came into the row the moment Preview was pressed, pushing the other buttons aside for work that may be over in 50 ms") + } + if clock.then == nil { + t.Fatal("the window asked for nothing later, so the face either never arrives or arrived at once") + } + if clock.after != window.BusyFaceAfter || clock.after <= 0 { + t.Errorf("the face was asked for after %v, and the delay this package names is %v", clock.after, window.BusyFaceAfter) + } + // And the state is there all the same: a second press starts nothing. + // The button is still live, so pressIfLive would press it - the refusal + // has to come from the state, see runner.onPreview. A second preview + // would ask the clock a second time. + press(t, content, text.ButtonPreview()) + if clock.asked != 1 { + t.Errorf("Preview was pressed twice inside the first moment and the clock was asked %d times, so the second press started a second preview", clock.asked) + } + + hold.look(func() { + // The work is still going and the clock fires: now the face goes on. + clock.fire() + if !box.Disabled() { + t.Error("the clock fired while the preview was still going and the form is not frozen") + } + cancel := buttonNamed(content, text.ButtonCancel()) + if cancel == nil || !cancel.Visible() || cancel.Disabled() { + t.Error("the clock fired while the preview was still going and there is no live Cancel to stop it") + } + if bar, _ := runMessages(content); bar != nil && bar.Visible() { + t.Error("a preview shows the progress bar, which stands at nought for as long as the preview takes - what a stuck run looks like") + } + }) + join(host) + + // Over: the face comes off at once, whether or not it was ever on. + if box.Disabled() { + t.Error("the preview is over and the form is still frozen") + } + if cancel := buttonNamed(content, text.ButtonCancel()); cancel != nil && cancel.Visible() { + t.Error("the preview is over and Cancel is still offered") + } +} + +// Work that is over before the clock fires never wears the face at all, and +// takes its request back so the clock cannot dress a screen that is idle. +func TestWorkOverBeforeTheClockWearsNoFace(t *testing.T) { + // No hold here, on purpose: the worker has to FINISH before the clock + // fires, which is the case this guard is about. A parked worker and a + // join is a guard that waits forever. + host := newFakeHost(t) + clock := host.holdTheClock() + window.Open(host) + if host.content == nil { + t.Fatal("opening the window put no screen in it") + } + content := tabNamed(t, host.content, text.TabOneTarget()) + w := test.NewWindow(host.content) + t.Cleanup(w.Close) + w.Resize(window.LargestOpening) + + fill(t, content, text.FieldOutputDir(), t.TempDir()) + box := entryUnder(t, content, text.FieldSize()) + if box == nil { + t.Fatal("there is no size box, so this guard read the wrong tree") + } + press(t, content, text.ButtonPreview()) + join(host) + if !clock.calledOff { + t.Error("the preview finished and the window did not call the face off, so a clock firing later would freeze an idle screen") + } + clock.fire() + if box.Disabled() { + t.Error("the clock fired after the preview was over and froze the form") + } + if cancel := buttonNamed(content, text.ButtonCancel()); cancel != nil && cancel.Visible() { + t.Error("the clock fired after the preview was over and offered Cancel for nothing") + } +} + +// The row of buttons is laid out again when a button comes or goes, so +// Preview and Generate stand where they stood once the work is over. +// +// The other half of the owner's report of 2026-09-21, and the lasting half: +// after a preview the two buttons stood 32 px to the left of centre and +// stayed there. Measured in the pinned toolkit: hiding a button changes what +// the row asks for, so the canvas lays out the row's PARENT, which hands the +// row the size it already had - and a container given its own size does +// nothing (fyne.Container.Resize). The row kept the positions it had worked +// out with Cancel in it. See busy.relay. +// +// The test driver lays nothing out on its own, so what is asked is the +// mechanism from both sides: a Cancel that has come is laid out - it has a +// size and stands after Generate - and once it has gone the two buttons are +// back where they were. A row left to the toolkit shows Cancel at no size +// on the way in and leaves the two buttons shifted on the way out. +func TestTheButtonsStandWhereTheyStoodAfterAPreview(t *testing.T) { + host, content, hold := heldScreen(t) + w := test.NewWindow(host.content) + t.Cleanup(w.Close) + w.Resize(window.LargestOpening) + settle(content, w) + + preview, generate := buttonNamed(content, text.ButtonPreview()), buttonNamed(content, text.ButtonGenerate()) + if preview == nil || generate == nil { + t.Fatal("the screen has no Preview or no Generate, so this guard read the wrong tree") + } + atRest := [2]fyne.Position{preview.Position(), generate.Position()} + if atRest[0].X <= 0 { + t.Fatal("Preview stands at the left edge, so the row has not been laid out and there is nothing to compare") + } + + fill(t, content, text.FieldOutputDir(), t.TempDir()) + fill(t, content, text.FieldCount(), "20000") + press(t, content, text.ButtonPreview()) + hold.look(func() { + cancel := buttonNamed(content, text.ButtonCancel()) + if cancel == nil || !cancel.Visible() { + t.Fatal("there is no Cancel while the preview is going, so the row never changed and this guard is about nothing") + } + if cancel.Size().Width <= 0 || cancel.Position().X <= generate.Position().X { + t.Errorf("Cancel came into the row and was never laid out: %v at %v, with Generate at %v", + cancel.Size(), cancel.Position(), generate.Position()) + } + if preview.Position() == atRest[0] { + t.Error("Cancel is in the row and Preview has not moved to make room, so the row was not laid out with it") + } + }) + join(host) + + if got := [2]fyne.Position{preview.Position(), generate.Position()}; got != atRest { + t.Errorf("after the preview Preview and Generate stand at %v and %v, and before it they stood at %v and %v - "+ + "the row kept the positions it worked out with Cancel in it", got[0], got[1], atRest[0], atRest[1]) + } +} + +// A face asked for by earlier work never dresses the work that came after. +// +// Calling the clock off is not enough, and an outside review of the pull +// request measured why: the real window's clock hands the face to the +// toolkit's queue (desktop.Later, time.AfterFunc then fyne.Do), and a face +// already queued when the work ends still runs. If the next press comes in +// that gap, the earlier face lands on the later work - a preview's face, +// which has no bar, on a run - and the run's own face finds the screen +// already dressed and does nothing, so the run has no progress bar at all. +// Each piece of work is an epoch, and a face checks it is still its own. +// +// Played out with the held clock: the preview's face is kept aside, the +// preview finishes, a run starts and is held, the stale face is fired as if +// the queue had just got round to it, and then the run's own face fires. +func TestAFaceAskedForByEarlierWorkNeverDressesLaterWork(t *testing.T) { + host, content, hold := heldScreen(t) + clock := host.holdTheClock() + w := test.NewWindow(host.content) + t.Cleanup(w.Close) + w.Resize(window.LargestOpening) + + dir := t.TempDir() + fill(t, content, text.FieldOutputDir(), dir) + fill(t, content, text.FieldSize(), "1kb") + fill(t, content, text.FieldCount(), "3") + + // The preview: its face is kept aside, and the preview is let through + // the hold and joined, which calls the clock off - too late for a face + // already on the toolkit's queue. + press(t, content, text.ButtonPreview()) + stale := clock.then + if stale == nil { + t.Fatal("the preview asked the clock for nothing, so there is no face to keep aside") + } + hold.look(func() {}) + join(host) + if !clock.calledOff { + t.Fatal("the preview finished without calling its face off, which is the case the guard above holds") + } + + // The run, held in flight, and the stale face arriving now - inside the + // hold, so the run is still going whatever the timing of three files. + hold.again() + press(t, content, text.ButtonGenerate()) + hold.look(func() { + stale() + if bar, _ := runMessages(content); bar != nil && bar.Visible() { + t.Error("the preview's face showed a bar, which is not its to show") + } + if cancel := buttonNamed(content, text.ButtonCancel()); cancel != nil && cancel.Visible() { + t.Error("the preview's face dressed the run that came after it") + } + // The run's own face, and what it earned: a bar. + clock.fire() + if bar, _ := runMessages(content); bar == nil || !bar.Visible() { + t.Error("the run's own face showed no bar - the stale face had dressed the screen first, and the run found it worn") + } + }) + join(host) +} diff --git a/internal/guard/controlstates_test.go b/internal/guard/controlstates_test.go index 809b15b..873b33c 100644 --- a/internal/guard/controlstates_test.go +++ b/internal/guard/controlstates_test.go @@ -1,10 +1,14 @@ package guard import ( + "image/color" "testing" "fyne.io/fyne/v2" + "fyne.io/fyne/v2/canvas" "fyne.io/fyne/v2/driver/desktop" + "fyne.io/fyne/v2/test" + "fyne.io/fyne/v2/theme" "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" ) @@ -211,3 +215,79 @@ func TestASegmentedSwitchMovesTheChoiceWithTheArrows(t *testing.T) { t.Errorf("Home left the switch on %q", s.Selected) } } + +// The tick of a switch is drawn on the whole of its square. +// +// Reported by the owner from the running window on 2026-09-21: the mark in a +// checked box was too small to read as one. Measured: the picture was drawn a +// step inside the square on every side, 12 px in a square of 20, and the +// toolkit's glyph fills about half of its own picture - so the tick was 7 px +// across. The glyph's own margin is all the room it needs, and this holds the +// picture to the square's edges. +func TestTheTickOfASwitchFillsItsSquare(t *testing.T) { + s := parts.NewToggle(func(bool) {}) + s.SetChecked(true) + s.Resize(s.MinSize()) + var square *canvas.Rectangle + var tick *canvas.Image + for _, o := range test.WidgetRenderer(s).Objects() { + switch drawn := o.(type) { + case *canvas.Rectangle: + if drawn.FillColor == parts.PaletteColour(theme.ColorNamePrimary, theme.VariantDark) { + square = drawn + } + case *canvas.Image: + tick = drawn + } + } + if square == nil || tick == nil { + t.Fatalf("a checked switch draws no filled square or no tick (square %v, tick %v)", square != nil, tick != nil) + } + if tick.Size() != square.Size() || tick.Position() != square.Position() { + t.Errorf("the tick is %v at %v and the square %v at %v - a tick drawn inside a margin of the square is a mark too small to read", + tick.Size(), tick.Position(), square.Size(), square.Position()) + } +} + +// A secondary button wears a face at rest, and the pointer lifts it. +// +// Reported by the owner from the running window on 2026-09-21: Duplicate, +// Choose, Preview and Add a batch looked very weak. Measured on the shot: an +// outline one pixel wide round nothing, with bold words inside - a bordered +// word, not a thing to press. The face is the surface a box to type in has, +// and it lightens under the pointer and again under a press, so the three +// states are three colours and not one. Asked of the drawn rectangle, so a +// face computed and not painted goes red. +func TestASecondaryButtonWearsAFaceAtRest(t *testing.T) { + b := parts.NewButton(parts.Secondary, "Preview", func() {}) + b.Resize(b.MinSize()) + faceOf := func() color.Color { + for _, o := range test.WidgetRenderer(b).Objects() { + if rect, is := o.(*canvas.Rectangle); is && rect.StrokeWidth > 0 { + return rect.FillColor + } + } + t.Fatal("the button draws no edged rectangle, so it has no face to measure") + return nil + } + rest := faceOf() + if want := parts.PaletteColour(theme.ColorNameInputBackground, theme.VariantDark); rest != want { + t.Errorf("at rest the face is %v and should be the surface of a box to type in, %v - an outline round nothing reads as a bordered word", rest, want) + } + b.MouseIn(&desktop.MouseEvent{}) + hovered := faceOf() + if hovered == rest { + t.Error("the pointer arriving changed nothing about the face, so nobody can see the button noticed it") + } + b.MouseDown(&desktop.MouseEvent{}) + if pressed := faceOf(); pressed == hovered || pressed == rest { + t.Errorf("a press draws %v, which is the hovered or the resting face - a press has to be told from the hover it follows", pressed) + } + b.MouseUp(&desktop.MouseEvent{}) + b.MouseOut() + b.Disable() + // No fill is nil or transparent, both of which the toolkit draws as nothing. + if off := faceOf(); off != nil && off != color.Transparent { + t.Errorf("a disabled button still wears a face (%v), and a face on a control that does nothing is a control that lies", off) + } +} diff --git a/internal/guard/detailpopup_test.go b/internal/guard/detailpopup_test.go index 2892b30..59ab703 100644 --- a/internal/guard/detailpopup_test.go +++ b/internal/guard/detailpopup_test.go @@ -5,7 +5,9 @@ import ( "testing" "fyne.io/fyne/v2" + "fyne.io/fyne/v2/canvas" "fyne.io/fyne/v2/test" + "fyne.io/fyne/v2/theme" "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" @@ -220,3 +222,51 @@ func namedOnScreen(o fyne.CanvasObject) string { } return "" } + +// The explanation floats on the surface an open list does, not on a panel's. +// +// Reported by the owner from the running window on 2026-09-21: the tooltips +// are hard to read because of their background. Measured on the shot: the box +// was drawn in the panel colour, and it opens over a panel - so it had no +// edge anywhere, and the sentence lay straight over the form covering the row +// beneath it. The palette's answer to "what floats over the form" was already +// in use by the list a menu drops down, and this holds the two to one surface. +// +// Held against the palette by name rather than against "not the panel": a box +// in any third colour would be told from the panel and still be a second +// floating surface nobody chose. +func TestTheExplanationFloatsOnTheSurfaceAnOpenListDoes(t *testing.T) { + app := test.NewApp() + defer test.NewApp() + app.Settings().SetTheme(parts.Theme()) + + content, c := laidOutWindow(t) + screen := tabNamed(t, content, text.TabOneTarget()) + button := detailButtonBeside(screen, text.FieldSize()) + if button == nil { + t.Fatalf("%q has no button that opens its explanation", text.FieldSize()) + } + test.MoveMouse(c, drawnCentre(t, button, text.FieldSize())) + box := button.Shown() + if box == nil { + t.Fatal("hovering the button put nothing on the sheet, so there is no box to measure") + } + + var surface *canvas.Rectangle + walk(box, func(o fyne.CanvasObject) { + if rect, is := o.(*canvas.Rectangle); is && surface == nil { + surface = rect + } + }) + if surface == nil { + t.Fatal("the explanation's box draws no rectangle, so it stands on nothing this guard can measure") + } + want := parts.PaletteColour(theme.ColorNameMenuBackground, theme.VariantDark) + if surface.FillColor != want { + t.Errorf("the explanation stands on %v and an open list on %v - a box the colour of the panel it opens over has no edge anywhere", + surface.FillColor, want) + } + if surface.CornerRadius != parts.RadiusField { + t.Errorf("the explanation's corner is %.0f and a floating control's is %d", surface.CornerRadius, parts.RadiusField) + } +} diff --git a/internal/guard/listedge_test.go b/internal/guard/listedge_test.go index 6663f76..4262638 100644 --- a/internal/guard/listedge_test.go +++ b/internal/guard/listedge_test.go @@ -142,3 +142,66 @@ func popUpIn(o fyne.CanvasObject) *widget.PopUp { }) return found } + +// A list opens downward whenever a few whole rows fit under its box, and +// upward only when fewer fit there than above it. +// +// Decision of the owner, 2026-09-21, from the running window: the format list +// on the preset screen opened UPWARD - over the question the preset asks - +// while the same list on the other screens opened downward, because the rule +// turned upward as soon as the list did not fit below and there was more room +// above. One control behaving two ways for a reason nobody could see. Now a +// list that has room for a few rows under its box goes there, shorter and +// scrolling, and upward is kept for a box standing just over the bar. +// +// Asked of the arithmetic directly, in rows, because the rule is about rows. +// The two guards above open real lists and hold the emergency half - a box at +// the foot still goes upward, and a cramped window still cuts the list. +func TestAListOpensDownwardWheneverAFewRowsFitUnderTheBox(t *testing.T) { + app := test.NewApp() + app.Settings().SetTheme(parts.Theme()) + t.Cleanup(func() { test.NewApp() }) + + row := parts.ListRowHeight() + const box = 31 + for _, tc := range []struct { + name string + canvas, top float32 + wantedRows float32 + wantRows float32 + wantDownward bool + whyItIsWorthA string + }{ + {"the preset screen's format box in the owner's window", 870, 557, 24, 9, true, + "274 px under the box holds nine rows, and that is where the list goes now"}, + {"a box with the whole list's room under it", 1300, 200, 10, 10, true, + "the ordinary case, unchanged"}, + {"a box against the foot of the window", 240, 240 - box, 24, 4, false, + "no row fits below, so upward is the only place (O113), and the ceiling of a 240 px window is four rows"}, + {"a few rows below and fewer above", 200, 60, 24, 3, true, + "three rows below beat one above, whichever side has more"}, + {"two rows below and sixteen above", 600, 480, 24, 10, false, + "fewer than the threshold below, and the ceiling of a 600 px window is ten rows"}, + } { + height, at := parts.RoomForList(tc.canvas, tc.top, box, tc.wantedRows*row) + downward := at >= tc.top+box + if downward != tc.wantDownward { + t.Errorf("%s: the list opens %s and should open %s (%s)", tc.name, + direction(downward), direction(tc.wantDownward), tc.whyItIsWorthA) + } + if height != tc.wantRows*row { + t.Errorf("%s: the list is %.0f px tall, which is %.2f rows, and should be %.0f rows (%s)", + tc.name, height, height/row, tc.wantRows, tc.whyItIsWorthA) + } + if !downward && at+height != tc.top { + t.Errorf("%s: a list opening upward ends at %.0f and the box starts at %.0f", tc.name, at+height, tc.top) + } + } +} + +func direction(downward bool) string { + if downward { + return "downward" + } + return "upward" +} diff --git a/internal/guard/listwords_test.go b/internal/guard/listwords_test.go index 6a74a8d..77e0d8a 100644 --- a/internal/guard/listwords_test.go +++ b/internal/guard/listwords_test.go @@ -13,8 +13,9 @@ import ( "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" ) -// The words in an open list start where the word in the box does, and the -// tick stands at the far end of the row. +// The words in an open list without pictures start where the word in the box +// does, with the tick at the far end of the row - and a list WITH pictures +// keeps its tick in front, the picture next and the words after it. // // Reported by the owner from the running window on 2026-09-16: the list of // formats looked right and the lists of outcomes and rules looked like words @@ -24,11 +25,19 @@ import ( // pictures in front of the formats made that look meant, and a list with no // picture and nothing chosen showed the empty column for what it was (O220). // +// The second half is the owner's too, from 2026-09-21: the tick moved to the +// end of EVERY row on 2026-09-16, and on the list of formats that pulled the +// picture and the word a column to the left of where they had stood - "what +// was nicely in the middle is at the left edge again". So a pictured row keeps +// the shape it had before O220 - tick, picture, words - and only a row with +// nothing to draw in front of its words starts them at the gutter. +// // Asked of two real lists on the screen: one without pictures, where the row's // words start at the gutter with nothing in front of them, and one with them, -// where the picture stands at the gutter and the tick behind the words. -// Positions are read off rows the list is actually drawing, because a row laid -// out at one width in a probe says nothing about the width the form gives it. +// where the tick's column stands at the gutter, the picture a column later +// and the words after it. Positions are read off rows the list is actually +// drawing, because a row laid out at one width in a probe says nothing about +// the width the form gives it. // // The rule is held against OUR geometry - the gutter, the tick, the picture - // and the distance to the box's own word is only logged. The first version @@ -64,26 +73,33 @@ func TestTheWordsInAnOpenListStartWhereTheWordInTheBoxDoes(t *testing.T) { } for _, row := range rows { words, tick, picture := piecesOfARow(t, row) - first := words.Position().X if tc.pictured { - first = picture.Position().X - } - if first != parts.RowGutter() { - t.Errorf("%s: row %q starts its first piece at %.1f rather than at the gutter (%.1f) - a column stands in front of the words and the list reads as words floating in a rectangle", - tc.field, row.Label(), first, parts.RowGutter()) + // Tick, picture, words: each starts where the one before it + // ends, a gap later, and the first of them at the gutter. + if tick.Position().X != parts.RowGutter() { + t.Errorf("%s: the tick of row %q stands at %.1f rather than at the gutter (%.1f) - the column that kept the picture and the word off the edge is gone", + tc.field, row.Label(), tick.Position().X, parts.RowGutter()) + } + if picture.Position().X <= tick.Position().X+tick.Size().Width { + t.Errorf("%s: the picture of row %q stands at %.1f, not after the tick's column ending at %.1f", + tc.field, row.Label(), picture.Position().X, tick.Position().X+tick.Size().Width) + } + if words.Position().X <= picture.Position().X+picture.Size().Width { + t.Errorf("%s: the words of row %q start at %.1f, not after the picture ending at %.1f", + tc.field, row.Label(), words.Position().X, picture.Position().X+picture.Size().Width) + } + continue } - if !tc.pictured { - t.Logf("%s: row %q words at %.1f, the box's word at %.1f (the toolkit's inset, logged and not held)", - tc.field, row.Label(), drv.AbsolutePositionForObject(words).X, boxWord) + if words.Position().X != parts.RowGutter() { + t.Errorf("%s: row %q starts its words at %.1f rather than at the gutter (%.1f) - a column stands in front of the words and the list reads as words floating in a rectangle", + tc.field, row.Label(), words.Position().X, parts.RowGutter()) } + t.Logf("%s: row %q words at %.1f, the box's word at %.1f (the toolkit's inset, logged and not held)", + tc.field, row.Label(), drv.AbsolutePositionForObject(words).X, boxWord) if tick.Position().X < words.Position().X+words.Size().Width { t.Errorf("%s: the tick of row %q stands at %.1f, in front of words ending at %.1f - the column it keeps pushes every list's words off the box's word", tc.field, row.Label(), tick.Position().X, words.Position().X+words.Size().Width) } - if tc.pictured && picture.Position().X+picture.Size().Width > words.Position().X { - t.Errorf("%s: the picture of row %q reaches %.1f, over words starting at %.1f", - tc.field, row.Label(), picture.Position().X+picture.Size().Width, words.Position().X) - } } list.TypedKey(&fyne.KeyEvent{Name: fyne.KeyEscape}) } diff --git a/internal/guard/pointerfocus_test.go b/internal/guard/pointerfocus_test.go index f9db904..06f9370 100644 --- a/internal/guard/pointerfocus_test.go +++ b/internal/guard/pointerfocus_test.go @@ -284,3 +284,87 @@ func chooserUnder(t *testing.T, o fyne.CanvasObject, label string) *parts.Choose } return chooser } + +// The window coming back to the front draws no mark nobody asked for, and +// draws again the one that was there. +// +// Reported by the owner on 2026-09-21 as one menu wearing a different colour +// from the others: on the single batch screen the format menu opened blue +// with a ring round it, and no other menu anywhere did. It was the keyboard +// mark, drawn by a call the control could not tell from the keyboard +// arriving - when the system gives the window the front, the driver calls +// FocusGained on whatever holds the keyboard (internal/driver/glfw/window.go +// processFocused, internal/app/focus_manager.go FocusGained), on the very +// first activation too, right after Open has put the keyboard on the first +// field quietly. So every start marked that one field, and every Alt-Tab +// back marked whichever control held the keyboard. +// +// The window says WindowReturning just before that call, from the toolkit's +// foreground hook - see internal/gui/run_cgo.go - and this guard does what +// the hook and the driver do, in that order, because the hook itself is +// behind cgo where no guard reaches it. Three arrivals, on the same control: +// the first activation after a quiet focus, a return after the keyboard had +// been used, and a plain FocusGained with no window in it, which is the +// keyboard moving here and has to draw as it always did. +func TestTheWindowComingBackDrawsOnlyTheMarkThatWasThere(t *testing.T) { + c, content := screenOnACanvas(t) + menu := chooserUnder(t, content, text.FieldFormat()) + + // The first activation: the window put the keyboard here quietly, and + // then the system handed it the front. + parts.FocusQuietly(c, menu) + if menu.Marked() { + t.Fatal("a quiet focus drew the mark, so this guard is not starting from the state it is about") + } + menu.FocusLost() + menu.WindowReturning() + menu.FocusGained() + if menu.Marked() { + t.Error("the window came to the front and the first menu drew the keyboard mark although nobody had pressed a key - " + + "the blue box the owner saw on one menu and no other") + } + + // The keyboard moving here, with no window in it, draws the mark as it + // always did - a return is not a licence to stay quiet afterwards. + // Asked while nothing is marked, so a return that was never forgotten + // answers with the old state and goes red here. + menu.FocusLost() + menu.FocusGained() + if !menu.Marked() { + t.Error("the keyboard arrived by a plain FocusGained and nothing was drawn, so the window's return has muted an ordinary arrival") + } + + // The keyboard has been used - the mark is drawn - and the window went + // behind and came back: what was drawn is drawn again. + menu.TypedKey(&fyne.KeyEvent{Name: fyne.KeyRight}) + if !menu.Marked() { + t.Fatal("an arrow key did not draw the mark, so the last part of this guard cannot start") + } + menu.FocusLost() + if menu.Marked() { + t.Fatal("losing the focus left the mark on") + } + menu.WindowReturning() + menu.FocusGained() + if !menu.Marked() { + t.Error("the window came back and the mark a keyboard user had was gone, so they no longer know which control is listening") + } + + // The same rule on the other controls that know who focused them, so the + // class is held and not the one instance the owner saw. + for _, control := range []interface { + fyne.Focusable + parts.Returnable + Marked() bool + }{ + checkNamed(content, text.FieldLabel()), + parts.NewSegments([]string{"one", "two"}, func(string) {}), + } { + control.FocusLost() + control.WindowReturning() + control.FocusGained() + if control.Marked() { + t.Errorf("%T drew its mark when the window came back although it had none before", control) + } + } +} diff --git a/internal/guard/remembered_test.go b/internal/guard/remembered_test.go index 8c347d4..ea8fdb7 100644 --- a/internal/guard/remembered_test.go +++ b/internal/guard/remembered_test.go @@ -178,16 +178,23 @@ func TestAFirstStartOpensAsTallAsTheScreensWantInTheMiddle(t *testing.T) { } } -// And what the screens want is what shows every work screen whole: laid out -// at the size Open hands back, no work screen scrolls - unless the ceiling -// stopped the window growing, which is the one reason a form may be cut. +// And what the screens want is what shows the FIRST work screen whole and +// nothing more: laid out at the size Open hands back, the screen the window +// opens on does not scroll and has no band of nothing under its form. +// +// The first screen alone, on the owner's decision of 2026-09-21. Until then +// the window opened as tall as the tallest work screen, so that none of the +// three scrolled from the first frame - and the one a person sees first, +// which is the shortest, opened with 70 px of nothing between its form and +// the bar. The batch screen is allowed to scroll on arrival now, which it +// does from the second batch on in any case. // // Asked of the real screens through Open rather than of HowToOpen alone, // because HowToOpen is arithmetic on two numbers and the number that matters -// is the one Open works out: a want that left out a screen, or forgot the -// strip above the screens, would pass every case above and still open a -// window whose batch screen scrolls from the first frame. -func TestTheFirstOpeningShowsEveryWorkScreenWhole(t *testing.T) { +// is the one Open works out: a want that forgot the strip above the screens +// would pass every case above and still open a window whose first screen +// scrolls from the first frame. +func TestTheFirstOpeningShowsTheFirstScreenWholeAndNoMore(t *testing.T) { ourTheme(t) host := newFakeHost(t) wanted := window.Open(host) @@ -199,45 +206,42 @@ func TestTheFirstOpeningShowsEveryWorkScreenWhole(t *testing.T) { w.Resize(size) if size.Height >= window.LargestOpening.Height { - t.Logf("the screens want %.0f px and the ceiling is %.0f, so a screen is allowed to scroll today", + t.Logf("the first screen wants %.0f px and the ceiling is %.0f, so it is allowed to scroll today", wanted.Height, window.LargestOpening.Height) } - checked := 0 - tightest := float32(-1) - for _, tab := range []string{text.TabOneTarget(), text.TabPresets(), text.TabRecipe()} { - screen := selectTab(t, host.content, tab) - w.Resize(fyne.NewSize(size.Width, size.Height-1)) - w.Resize(size) - scroll := scrollIn(screen) - if scroll == nil { - t.Fatalf("the %s screen has no scroll, so this guard cannot say whether it fits", tab) - } - form, room := scroll.Content.MinSize().Height, scroll.Size().Height - if form > room && size.Height < window.LargestOpening.Height { - t.Errorf("at the first opening of %v the %s screen's form needs %.0f px and gets %.0f, so it"+ - " scrolls from the first frame although the window had room to grow", size, tab, form, room) - } - if slack := room - form; tightest < 0 || slack < tightest { - tightest = slack - } - checked++ + screen := selectTab(t, host.content, text.TabOneTarget()) + w.Resize(fyne.NewSize(size.Width, size.Height-1)) + w.Resize(size) + scroll := scrollIn(screen) + if scroll == nil { + t.Fatalf("the %s screen has no scroll, so this guard cannot say whether it fits", text.TabOneTarget()) } - if checked != 3 { - t.Fatalf("checked %d screens, and there are three work screens", checked) + form, room := scroll.Content.MinSize().Height, scroll.Size().Height + if form > room && size.Height < window.LargestOpening.Height { + t.Errorf("at the first opening of %v the first screen's form needs %.0f px and gets %.0f, so it"+ + " scrolls from the first frame although the window had room to grow", size, form, room) } - // And no taller than that. The tallest screen fits with nothing to spare, + // And no taller than that. The first screen fits with nothing to spare, // because the height is worked out from it - a window that opened taller // would be the band of nothing under the form that O202 is about, back // under another number. Asked at the ceiling too: a want a hundred pixels // over the truth is stopped by the ceiling and leaves the band all the - // same, and the first version of this line let that through by excusing - // everything at the ceiling. Only a form TALLER than the room is the - // ceiling's doing. - if tightest > 1 { - t.Errorf("the first opening is %v and the tallest work screen still has %.0f px to spare under"+ - " its form, so the window opens taller than the screens want", size, tightest) + // same. Only a form TALLER than the room is the ceiling's doing. + if slack := room - form; slack > 1 { + t.Errorf("the first opening is %v and the first screen still has %.0f px to spare under"+ + " its form, so the window opens taller than the screen it opens on wants", size, slack) + } + // The other two are not what the height is worked out from, and the + // batch screen is taller than the first - so a window sized for the first + // screen that showed the batch screen whole as well would be a window + // sized for the batch screen after all, which is the band coming back. + batches := selectTab(t, host.content, text.TabRecipe()) + w.Resize(fyne.NewSize(size.Width, size.Height-1)) + w.Resize(size) + if batchScroll := scrollIn(batches); batchScroll != nil { + t.Logf("first opening %v: the first screen shows whole with %.2f px to spare, the batch screen has %.0f px of form in %.0f px of room", + size, room-form, batchScroll.Content.MinSize().Height, batchScroll.Size().Height) } - t.Logf("first opening %v: every work screen shows whole, the tallest with %.2f px to spare", size, tightest) } // A size with a nought in it is refused at BOTH ends by one predicate. diff --git a/internal/guard/runstate_test.go b/internal/guard/runstate_test.go index 0dfea6d..380619f 100644 --- a/internal/guard/runstate_test.go +++ b/internal/guard/runstate_test.go @@ -150,7 +150,7 @@ func TestAPreviewDoesItsDiskWorkOffTheInterfaceThread(t *testing.T) { worker := strings.Index(fn, "go func()") call := strings.Index(fn, "engine.Run(") - busy := strings.Index(fn, "r.setBusy(true") + busy := strings.Index(fn, "r.busy.set(true") switch { case call < 0: t.Fatal("onPreview no longer goes through engine.Run, so a preview has stopped answering the same question the run does") diff --git a/internal/guard/testdata/screens/catalogue.png b/internal/guard/testdata/screens/catalogue.png index 4021657..9380854 100644 Binary files a/internal/guard/testdata/screens/catalogue.png and b/internal/guard/testdata/screens/catalogue.png differ diff --git a/internal/guard/testdata/screens/catalogue.xml b/internal/guard/testdata/screens/catalogue.xml index 88b4921..d254dae 100644 --- a/internal/guard/testdata/screens/catalogue.xml +++ b/internal/guard/testdata/screens/catalogue.xml @@ -1,7 +1,7 @@ - + - - + + @@ -114,7 +114,7 @@ - + Generate @@ -132,7 +132,7 @@ - + Generate @@ -150,7 +150,7 @@ - + Generate @@ -168,7 +168,7 @@ - + Generate @@ -294,7 +294,7 @@ - + Write a label inside each generated file, including the ones that are far too small to hold it @@ -698,7 +698,7 @@ - + @@ -735,7 +735,7 @@ - + @@ -1136,86 +1136,86 @@ - - avif + + avif - - bmp + + bmp - - csv + + csv - - - docx + + + docx - - gif + + gif - - html + + html - - ico + + ico - - jpg + + jpg - - json + + json - - jxl + + jxl - - log + + log - - md + + md @@ -1729,7 +1729,7 @@ - + Remove @@ -1774,7 +1774,7 @@ - + Remove @@ -1821,7 +1821,7 @@ - + Choose... @@ -2074,21 +2074,23 @@ - - - - - - - worked out from the size - - - + + + + + + + + worked out from the size + + + + - - + + @@ -2329,7 +2331,7 @@ - + @@ -3331,7 +3333,7 @@ - + Preview @@ -3423,12 +3425,12 @@ - - - - + + + + Not drawn, and why - + @@ -3514,6 +3516,18 @@ • + + + + + Returnable - an interface: a control the window can tell it is coming back to the front, nothing of its own to draw + + + + + • + + @@ -3529,12 +3543,12 @@ - - - - + + + + Layout only, and why - + @@ -3612,7 +3626,7 @@ - Padded - one distance of the scale round its content + Text - the width a short piece of text needs - a name, a template, a file name @@ -3624,7 +3638,7 @@ - Stacked - panels one under another + Padded - one distance of the scale round its content @@ -3636,7 +3650,7 @@ - Screen - the readable width round a head and its sections + Stacked - panels one under another @@ -3648,7 +3662,7 @@ - BesideFields - the room to the right of the column of names + Screen - the readable width round a head and its sections @@ -3660,7 +3674,7 @@ - WithRoomForARun - the room under a form for the bar to speak into + BesideFields - the room to the right of the column of names @@ -3672,7 +3686,7 @@ - Flush - a label kept by a screen, on the edge every other word stands on + WithRoomForARun - the room under a form for the bar to speak into @@ -3680,6 +3694,18 @@ • + + + + + Flush - a label kept by a screen, on the edge every other word stands on + + + + + • + + diff --git a/internal/guard/testdata/screens/generate-chosen-by-key.png b/internal/guard/testdata/screens/generate-chosen-by-key.png index 6d69020..16d4799 100644 Binary files a/internal/guard/testdata/screens/generate-chosen-by-key.png and b/internal/guard/testdata/screens/generate-chosen-by-key.png differ diff --git a/internal/guard/testdata/screens/generate-chosen-by-key.xml b/internal/guard/testdata/screens/generate-chosen-by-key.xml index d3520c3..53c090a 100644 --- a/internal/guard/testdata/screens/generate-chosen-by-key.xml +++ b/internal/guard/testdata/screens/generate-chosen-by-key.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -305,7 +309,7 @@ - + Choose... @@ -358,7 +362,7 @@ - + @@ -379,7 +383,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate-chosen.png b/internal/guard/testdata/screens/generate-chosen.png index 11d6438..8bee73b 100644 Binary files a/internal/guard/testdata/screens/generate-chosen.png and b/internal/guard/testdata/screens/generate-chosen.png differ diff --git a/internal/guard/testdata/screens/generate-chosen.xml b/internal/guard/testdata/screens/generate-chosen.xml index acf2af5..7237440 100644 --- a/internal/guard/testdata/screens/generate-chosen.xml +++ b/internal/guard/testdata/screens/generate-chosen.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -305,7 +309,7 @@ - + Choose... @@ -358,7 +362,7 @@ - + @@ -379,7 +383,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate-empty.png b/internal/guard/testdata/screens/generate-empty.png index 7b63f59..7064173 100644 Binary files a/internal/guard/testdata/screens/generate-empty.png and b/internal/guard/testdata/screens/generate-empty.png differ diff --git a/internal/guard/testdata/screens/generate-empty.xml b/internal/guard/testdata/screens/generate-empty.xml index 8206ccb..83317d5 100644 --- a/internal/guard/testdata/screens/generate-empty.xml +++ b/internal/guard/testdata/screens/generate-empty.xml @@ -181,18 +181,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -207,21 +209,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -318,7 +322,7 @@ - + Choose... @@ -371,7 +375,7 @@ - + @@ -392,7 +396,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate-focused.png b/internal/guard/testdata/screens/generate-focused.png index f6c0026..0cca0cb 100644 Binary files a/internal/guard/testdata/screens/generate-focused.png and b/internal/guard/testdata/screens/generate-focused.png differ diff --git a/internal/guard/testdata/screens/generate-focused.xml b/internal/guard/testdata/screens/generate-focused.xml index b67fb6c..383d277 100644 --- a/internal/guard/testdata/screens/generate-focused.xml +++ b/internal/guard/testdata/screens/generate-focused.xml @@ -169,18 +169,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -195,21 +197,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -306,7 +310,7 @@ - + Choose... @@ -359,7 +363,7 @@ - + @@ -380,7 +384,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate-hovered.png b/internal/guard/testdata/screens/generate-hovered.png index 3f720d0..7ed66e3 100644 Binary files a/internal/guard/testdata/screens/generate-hovered.png and b/internal/guard/testdata/screens/generate-hovered.png differ diff --git a/internal/guard/testdata/screens/generate-hovered.xml b/internal/guard/testdata/screens/generate-hovered.xml index b577423..d56ecaf 100644 --- a/internal/guard/testdata/screens/generate-hovered.xml +++ b/internal/guard/testdata/screens/generate-hovered.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -305,7 +309,7 @@ - + Choose... @@ -358,7 +362,7 @@ - + @@ -379,7 +383,7 @@ - + Preview @@ -422,7 +426,7 @@ - + diff --git a/internal/guard/testdata/screens/generate-menu-hovered.png b/internal/guard/testdata/screens/generate-menu-hovered.png index 652dd0d..4398f6e 100644 Binary files a/internal/guard/testdata/screens/generate-menu-hovered.png and b/internal/guard/testdata/screens/generate-menu-hovered.png differ diff --git a/internal/guard/testdata/screens/generate-menu-hovered.xml b/internal/guard/testdata/screens/generate-menu-hovered.xml index 3363ea1..c8397f7 100644 --- a/internal/guard/testdata/screens/generate-menu-hovered.xml +++ b/internal/guard/testdata/screens/generate-menu-hovered.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -305,7 +309,7 @@ - + Choose... @@ -358,7 +362,7 @@ - + @@ -379,7 +383,7 @@ - + Preview @@ -440,170 +444,170 @@ - - - avif + + + avif - - bmp + + bmp - - csv + + csv - - docx + + docx - - gif + + gif - - html + + html - - ico + + ico - - jpg + + jpg - - json + + json - - jxl + + jxl - - log + + log - - md + + md - - pdf + + pdf - - png + + png - - pptx + + pptx - - svg + + svg - - targz + + targz - - tiff + + tiff - - txt + + txt - - wav + + wav - - webp + + webp - - xlsx + + xlsx - - xml + + xml - - zip + + zip diff --git a/internal/guard/testdata/screens/generate-menu-keyed.png b/internal/guard/testdata/screens/generate-menu-keyed.png index 5958ca3..bb9e1ec 100644 Binary files a/internal/guard/testdata/screens/generate-menu-keyed.png and b/internal/guard/testdata/screens/generate-menu-keyed.png differ diff --git a/internal/guard/testdata/screens/generate-menu-keyed.xml b/internal/guard/testdata/screens/generate-menu-keyed.xml index 3d38bb2..86419fb 100644 --- a/internal/guard/testdata/screens/generate-menu-keyed.xml +++ b/internal/guard/testdata/screens/generate-menu-keyed.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -305,7 +309,7 @@ - + Choose... @@ -358,7 +362,7 @@ - + @@ -379,7 +383,7 @@ - + Preview @@ -440,170 +444,170 @@ - - - avif + + + avif - - bmp + + bmp - - csv + + csv - - docx + + docx - - gif + + gif - - html + + html - - ico + + ico - - jpg + + jpg - - json + + json - - jxl + + jxl - - log + + log - - md + + md - - pdf + + pdf - - png + + png - - pptx + + pptx - - svg + + svg - - targz + + targz - - tiff + + tiff - - txt + + txt - - wav + + wav - - webp + + webp - - xlsx + + xlsx - - xml + + xml - - zip + + zip diff --git a/internal/guard/testdata/screens/generate-menu.png b/internal/guard/testdata/screens/generate-menu.png index c6907c2..f1f62b3 100644 Binary files a/internal/guard/testdata/screens/generate-menu.png and b/internal/guard/testdata/screens/generate-menu.png differ diff --git a/internal/guard/testdata/screens/generate-menu.xml b/internal/guard/testdata/screens/generate-menu.xml index e2b0f5c..3c95915 100644 --- a/internal/guard/testdata/screens/generate-menu.xml +++ b/internal/guard/testdata/screens/generate-menu.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -305,7 +309,7 @@ - + Choose... @@ -358,7 +362,7 @@ - + @@ -379,7 +383,7 @@ - + Preview @@ -440,170 +444,170 @@ - - - avif + + + avif - - bmp + + bmp - - csv + + csv - - docx + + docx - - gif + + gif - - html + + html - - ico + + ico - - jpg + + jpg - - json + + json - - jxl + + jxl - - log + + log - - md + + md - - pdf + + pdf - - png + + png - - pptx + + pptx - - svg + + svg - - targz + + targz - - tiff + + tiff - - txt + + txt - - wav + + wav - - webp + + webp - - xlsx + + xlsx - - xml + + xml - - zip + + zip diff --git a/internal/guard/testdata/screens/generate-refused-both.png b/internal/guard/testdata/screens/generate-refused-both.png index 48f54e0..476f4be 100644 Binary files a/internal/guard/testdata/screens/generate-refused-both.png and b/internal/guard/testdata/screens/generate-refused-both.png differ diff --git a/internal/guard/testdata/screens/generate-refused-both.xml b/internal/guard/testdata/screens/generate-refused-both.xml index a7e769c..187b69c 100644 --- a/internal/guard/testdata/screens/generate-refused-both.xml +++ b/internal/guard/testdata/screens/generate-refused-both.xml @@ -194,18 +194,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -220,21 +222,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -331,7 +335,7 @@ - + Choose... @@ -384,7 +388,7 @@ - + @@ -405,7 +409,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate-refused-setting.png b/internal/guard/testdata/screens/generate-refused-setting.png index 1bbbeee..9b2af8c 100644 Binary files a/internal/guard/testdata/screens/generate-refused-setting.png and b/internal/guard/testdata/screens/generate-refused-setting.png differ diff --git a/internal/guard/testdata/screens/generate-refused-setting.xml b/internal/guard/testdata/screens/generate-refused-setting.xml index 3d7bdd7..641540e 100644 --- a/internal/guard/testdata/screens/generate-refused-setting.xml +++ b/internal/guard/testdata/screens/generate-refused-setting.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -388,7 +392,7 @@ - + Choose... @@ -441,7 +445,7 @@ - + @@ -462,7 +466,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate-refused.png b/internal/guard/testdata/screens/generate-refused.png index df830d5..e496e4f 100644 Binary files a/internal/guard/testdata/screens/generate-refused.png and b/internal/guard/testdata/screens/generate-refused.png differ diff --git a/internal/guard/testdata/screens/generate-refused.xml b/internal/guard/testdata/screens/generate-refused.xml index 4d7c863..f4ef311 100644 --- a/internal/guard/testdata/screens/generate-refused.xml +++ b/internal/guard/testdata/screens/generate-refused.xml @@ -183,18 +183,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -209,21 +211,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -320,7 +324,7 @@ - + Choose... @@ -373,7 +377,7 @@ - + @@ -394,7 +398,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate-switch-by-key.png b/internal/guard/testdata/screens/generate-switch-by-key.png index 6f390e4..277c21c 100644 Binary files a/internal/guard/testdata/screens/generate-switch-by-key.png and b/internal/guard/testdata/screens/generate-switch-by-key.png differ diff --git a/internal/guard/testdata/screens/generate-switch-by-key.xml b/internal/guard/testdata/screens/generate-switch-by-key.xml index 30f3845..33818c1 100644 --- a/internal/guard/testdata/screens/generate-switch-by-key.xml +++ b/internal/guard/testdata/screens/generate-switch-by-key.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -305,7 +309,7 @@ - + Choose... @@ -358,7 +362,7 @@ - + @@ -379,7 +383,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate-typed.png b/internal/guard/testdata/screens/generate-typed.png index d6e64c8..9ccc44f 100644 Binary files a/internal/guard/testdata/screens/generate-typed.png and b/internal/guard/testdata/screens/generate-typed.png differ diff --git a/internal/guard/testdata/screens/generate-typed.xml b/internal/guard/testdata/screens/generate-typed.xml index e892b4d..0c43239 100644 --- a/internal/guard/testdata/screens/generate-typed.xml +++ b/internal/guard/testdata/screens/generate-typed.xml @@ -180,18 +180,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -206,21 +208,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -317,7 +321,7 @@ - + Choose... @@ -370,7 +374,7 @@ - + @@ -391,7 +395,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate-unchecked.png b/internal/guard/testdata/screens/generate-unchecked.png index b5dcbc4..29c277e 100644 Binary files a/internal/guard/testdata/screens/generate-unchecked.png and b/internal/guard/testdata/screens/generate-unchecked.png differ diff --git a/internal/guard/testdata/screens/generate-unchecked.xml b/internal/guard/testdata/screens/generate-unchecked.xml index 44cf4e6..ac44356 100644 --- a/internal/guard/testdata/screens/generate-unchecked.xml +++ b/internal/guard/testdata/screens/generate-unchecked.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -305,7 +309,7 @@ - + Choose... @@ -378,7 +382,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/generate.png b/internal/guard/testdata/screens/generate.png index 610093c..37507c1 100644 Binary files a/internal/guard/testdata/screens/generate.png and b/internal/guard/testdata/screens/generate.png differ diff --git a/internal/guard/testdata/screens/generate.xml b/internal/guard/testdata/screens/generate.xml index 9dfc5f6..5e126b5 100644 --- a/internal/guard/testdata/screens/generate.xml +++ b/internal/guard/testdata/screens/generate.xml @@ -168,18 +168,20 @@ - - - - - - - files + + + + + + + + files + - - + + @@ -194,21 +196,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -305,7 +309,7 @@ - + Choose... @@ -358,7 +362,7 @@ - + @@ -379,7 +383,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/preset-menu-setting.png b/internal/guard/testdata/screens/preset-menu-setting.png index b2a18a5..d5ce549 100644 Binary files a/internal/guard/testdata/screens/preset-menu-setting.png and b/internal/guard/testdata/screens/preset-menu-setting.png differ diff --git a/internal/guard/testdata/screens/preset-menu-setting.xml b/internal/guard/testdata/screens/preset-menu-setting.xml index 3f8daaf..5da9d42 100644 --- a/internal/guard/testdata/screens/preset-menu-setting.xml +++ b/internal/guard/testdata/screens/preset-menu-setting.xml @@ -206,21 +206,23 @@ - - - - - - - 1B,1kb,1mb - - - + + + + + + + + 1B,1kb,1mb + + + + - - + + @@ -288,7 +290,7 @@ - + Choose... @@ -344,7 +346,7 @@ - + Preview @@ -405,170 +407,170 @@ - - avif + + avif - - bmp + + bmp - - csv + + csv - - docx + + docx - - gif + + gif - - html + + html - - ico + + ico - - jpg + + jpg - - json + + json - - jxl + + jxl - - log + + log - - md + + md - - - pdf + + + pdf - - png + + png - - pptx + + pptx - - svg + + svg - - targz + + targz - - tiff + + tiff - - txt + + txt - - wav + + wav - - webp + + webp - - xlsx + + xlsx - - xml + + xml - - zip + + zip diff --git a/internal/guard/testdata/screens/preset-menu.png b/internal/guard/testdata/screens/preset-menu.png index b7c3e54..091c1fe 100644 Binary files a/internal/guard/testdata/screens/preset-menu.png and b/internal/guard/testdata/screens/preset-menu.png differ diff --git a/internal/guard/testdata/screens/preset-menu.xml b/internal/guard/testdata/screens/preset-menu.xml index 0493f2e..08a3ddc 100644 --- a/internal/guard/testdata/screens/preset-menu.xml +++ b/internal/guard/testdata/screens/preset-menu.xml @@ -206,21 +206,23 @@ - - - - - - - 1B,1kb,1mb - - - + + + + + + + + 1B,1kb,1mb + + + + - - + + @@ -288,7 +290,7 @@ - + Choose... @@ -344,7 +346,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/preset-refused.png b/internal/guard/testdata/screens/preset-refused.png index a27a538..901a986 100644 Binary files a/internal/guard/testdata/screens/preset-refused.png and b/internal/guard/testdata/screens/preset-refused.png differ diff --git a/internal/guard/testdata/screens/preset-refused.xml b/internal/guard/testdata/screens/preset-refused.xml index 6ac73b0..c9a49f5 100644 --- a/internal/guard/testdata/screens/preset-refused.xml +++ b/internal/guard/testdata/screens/preset-refused.xml @@ -218,21 +218,23 @@ - - - - - - - 1B,1kb,1mb - - - + + + + + + + + 1B,1kb,1mb + + + + - - + + @@ -300,7 +302,7 @@ - + Choose... @@ -356,7 +358,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/preset.png b/internal/guard/testdata/screens/preset.png index 1c2e36b..c9c1f32 100644 Binary files a/internal/guard/testdata/screens/preset.png and b/internal/guard/testdata/screens/preset.png differ diff --git a/internal/guard/testdata/screens/preset.xml b/internal/guard/testdata/screens/preset.xml index 19ec315..5a994a8 100644 --- a/internal/guard/testdata/screens/preset.xml +++ b/internal/guard/testdata/screens/preset.xml @@ -206,21 +206,23 @@ - - - - - - - 1B,1kb,1mb - - - + + + + + + + + 1B,1kb,1mb + + + + - - + + @@ -288,7 +290,7 @@ - + Choose... @@ -344,7 +346,7 @@ - + Preview diff --git a/internal/guard/testdata/screens/recipe-contents.png b/internal/guard/testdata/screens/recipe-contents.png index 783f2f5..f3d64d2 100644 Binary files a/internal/guard/testdata/screens/recipe-contents.png and b/internal/guard/testdata/screens/recipe-contents.png differ diff --git a/internal/guard/testdata/screens/recipe-contents.xml b/internal/guard/testdata/screens/recipe-contents.xml index d5a56a8..c1aa9d0 100644 --- a/internal/guard/testdata/screens/recipe-contents.xml +++ b/internal/guard/testdata/screens/recipe-contents.xml @@ -85,7 +85,7 @@ - + Duplicate @@ -133,21 +133,23 @@ - - - - - - - - - - + + + + + + + + + + + + - - + + @@ -245,21 +247,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -391,7 +395,7 @@ - + Remove @@ -401,7 +405,7 @@ - + Add files inside @@ -445,7 +449,7 @@ - + Choose... @@ -464,21 +468,23 @@ - - - - - - - manifest.json - - - + + + + + + + + manifest.json + + + + - - + + @@ -548,7 +554,7 @@ - + Preview @@ -587,7 +593,7 @@ - + Add a batch diff --git a/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.png b/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.png index 5675329..6ea8c37 100644 Binary files a/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.png and b/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.png differ diff --git a/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.xml b/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.xml index 978c416..d2043cd 100644 --- a/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.xml +++ b/internal/guard/testdata/screens/recipe-refused-with-one-batch-filled.xml @@ -85,12 +85,12 @@ - + Duplicate - + Remove @@ -138,22 +138,24 @@ - - - - - - - - - - + + + + + + + + + + + + + - - - + + @@ -279,21 +281,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -380,12 +384,12 @@ - + Duplicate - + Remove @@ -433,18 +437,20 @@ - - - - - - - second + + + + + + + + second + - - + + @@ -539,21 +545,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -649,7 +657,7 @@ - + Choose... @@ -668,21 +676,23 @@ - - - - - - - manifest.json - - - + + + + + + + + manifest.json + + + + - - + + @@ -760,7 +770,7 @@ - + Preview @@ -799,7 +809,7 @@ - + Add a batch diff --git a/internal/guard/testdata/screens/recipe-refused.png b/internal/guard/testdata/screens/recipe-refused.png index e898d68..c6eec40 100644 Binary files a/internal/guard/testdata/screens/recipe-refused.png and b/internal/guard/testdata/screens/recipe-refused.png differ diff --git a/internal/guard/testdata/screens/recipe-refused.xml b/internal/guard/testdata/screens/recipe-refused.xml index 5bbfffc..48ce42d 100644 --- a/internal/guard/testdata/screens/recipe-refused.xml +++ b/internal/guard/testdata/screens/recipe-refused.xml @@ -85,7 +85,7 @@ - + Duplicate @@ -133,22 +133,24 @@ - - - - - - - - - - + + + + + + + + + + + + + - - - + + @@ -274,21 +276,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -384,7 +388,7 @@ - + Choose... @@ -403,21 +407,23 @@ - - - - - - - manifest.json - - - + + + + + + + + manifest.json + + + + - - + + @@ -487,7 +493,7 @@ - + Preview @@ -526,7 +532,7 @@ - + Add a batch diff --git a/internal/guard/testdata/screens/recipe-two-batches.png b/internal/guard/testdata/screens/recipe-two-batches.png index d304ef8..fcd2d6f 100644 Binary files a/internal/guard/testdata/screens/recipe-two-batches.png and b/internal/guard/testdata/screens/recipe-two-batches.png differ diff --git a/internal/guard/testdata/screens/recipe-two-batches.xml b/internal/guard/testdata/screens/recipe-two-batches.xml index 591b16c..a359830 100644 --- a/internal/guard/testdata/screens/recipe-two-batches.xml +++ b/internal/guard/testdata/screens/recipe-two-batches.xml @@ -85,12 +85,12 @@ - + Duplicate - + Remove @@ -138,21 +138,23 @@ - - - - - - - - - - + + + + + + + + + + + + - - + + @@ -250,21 +252,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -351,12 +355,12 @@ - + Duplicate - + Remove @@ -404,21 +408,23 @@ - - - - - - - - - - + + + + + + + + + + + + - - + + @@ -516,21 +522,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -626,7 +634,7 @@ - + Choose... @@ -645,21 +653,23 @@ - - - - - - - manifest.json - - - + + + + + + + + manifest.json + + + + - - + + @@ -737,7 +747,7 @@ - + Preview @@ -776,7 +786,7 @@ - + Add a batch diff --git a/internal/guard/testdata/screens/recipe.png b/internal/guard/testdata/screens/recipe.png index 490ce99..34608dd 100644 Binary files a/internal/guard/testdata/screens/recipe.png and b/internal/guard/testdata/screens/recipe.png differ diff --git a/internal/guard/testdata/screens/recipe.xml b/internal/guard/testdata/screens/recipe.xml index a78e152..0098f46 100644 --- a/internal/guard/testdata/screens/recipe.xml +++ b/internal/guard/testdata/screens/recipe.xml @@ -85,7 +85,7 @@ - + Duplicate @@ -133,21 +133,23 @@ - - - - - - - - - - + + + + + + + + + + + + - - + + @@ -245,21 +247,23 @@ - - - - - - - files_0001 - - - + + + + + + + + files_0001 + + + + - - + + @@ -355,7 +359,7 @@ - + Choose... @@ -374,21 +378,23 @@ - - - - - - - manifest.json - - - + + + + + + + + manifest.json + + + + - - + + @@ -458,7 +464,7 @@ - + Preview @@ -497,7 +503,7 @@ - + Add a batch diff --git a/internal/guard/typeshape_test.go b/internal/guard/typeshape_test.go index 23dfff7..65db5e3 100644 --- a/internal/guard/typeshape_test.go +++ b/internal/guard/typeshape_test.go @@ -35,11 +35,14 @@ import ( // Down is routine. Up is the owner's decision, the same as every other ceiling // in this package. const ( - // Measured 2026-09-07. Both are internal/gui/window.runner, which is the - // screen that drives a run - it holds the widgets, the progress state and - // the cancel plumbing at once. - mostMethods = 28 - mostFields = 22 + // Measured 2026-09-21. Until then both were internal/gui/window.runner, + // which is the screen that drives a run - it held the widgets, the + // progress state and the cancel plumbing at once, and stood ON both + // ceilings. The busy state and its four controls left it that day for a + // type of their own (runbusy.go), and the ratchet moved down to the next + // widest: parts.Fields by methods, window.batch by fields. + mostMethods = 27 + mostFields = 21 // What counts as crowding, in the shape this package already uses // everywhere else: an ABSOLUTE number rather than a percentage of the diff --git a/internal/guard/window_test.go b/internal/guard/window_test.go index 55dd8d6..ad89e59 100644 --- a/internal/guard/window_test.go +++ b/internal/guard/window_test.go @@ -6,6 +6,7 @@ import ( "strings" "sync" "testing" + "time" "fyne.io/fyne/v2" "fyne.io/fyne/v2/canvas" @@ -83,6 +84,31 @@ type fakeHost struct { // hold parks the worker just before it reports, so that a guard can read // the screen while a run is going. Nil unless a guard asked for one. hold *holdDuringRun + + // held is what the window asked to run later and has not run yet, when a + // guard is holding the clock - see Later. Nil means the clock runs at once. + held *heldClock +} + +// heldClock keeps what the window asked for later, so a guard can look at the +// screen BEFORE the busy face arrives and then let it arrive. +type heldClock struct { + after time.Duration + then func() + // calledOff is whether the window took the request back before it fired, + // and asked how many times the window asked at all. + calledOff bool + asked int +} + +// fire runs what was held, once, unless it was called off. +func (c *heldClock) fire() { + if c.calledOff || c.then == nil { + return + } + then := c.then + c.then = nil + then() } // newFakeHost builds one and promises that nothing it starts outlives the test. @@ -132,6 +158,30 @@ func (h *fakeHost) SetContent(o fyne.CanvasObject) { } func (h *fakeHost) SetCloseIntercept(fn func()) { h.intercept = fn } +// Later is the clock the busy face waits on. At once, unless a guard holds +// the clock: under the test driver fyne.Do runs on the calling goroutine, so +// a real timer would be a second writer to the widgets a guard is reading, +// and every guard that ran before the face had a delay saw the face at once +// - so at once is what keeps them asking what they asked. A guard that wants +// to see the moment before the face holds the clock (holdTheClock) and fires +// it when it is ready. +func (h *fakeHost) Later(after time.Duration, then func()) func() { + if h.held == nil { + then() + return func() {} + } + h.held.after, h.held.then, h.held.calledOff = after, then, false + h.held.asked++ + return func() { h.held.calledOff = true } +} + +// holdTheClock makes this host keep what the window asks for later, rather +// than running it at once. +func (h *fakeHost) holdTheClock() *heldClock { + h.held = &heldClock{} + return h.held +} + // SetWaitForWork is the optional interface the window offers rather than // requires - a real window has no use for it and does not implement it. func (h *fakeHost) SetWaitForWork(fn func()) { h.waitForWork = fn } @@ -208,6 +258,17 @@ func (p *holdDuringRun) look(fn func()) { // a test that failed earlier. func (p *holdDuringRun) free() { p.freed.Do(func() { close(p.release) }) } +// again arms the hold for a second piece of work, for a guard that has to +// see one run in flight AFTER another has finished. Only after the first +// has been joined: the worker captured enter as a method value at Open, so +// fresh channels and a fresh once under the same pointer are what it parks +// on next, and resetting them while a worker still holds them would be the +// race the hold exists to prevent. +func (p *holdDuringRun) again() { + p.reached, p.release = make(chan struct{}, 1), make(chan struct{}) + p.parked, p.freed = sync.Once{}, sync.Once{} +} + // picked is what the stand in answers when a screen asks where the files // should go, and asked counts how often it was asked. A real picker needs a // real window, and the behaviour worth proving is that the button reaches one diff --git a/internal/guard/windowreturning_test.go b/internal/guard/windowreturning_test.go new file mode 100644 index 0000000..af599e7 --- /dev/null +++ b/internal/guard/windowreturning_test.go @@ -0,0 +1,174 @@ +package guard + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "path/filepath" + "strings" + "testing" + + "fyne.io/fyne/v2" + + "github.com/donislawdev/TestingFilesGenerator/internal/gui" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" + "github.com/donislawdev/TestingFilesGenerator/internal/gui/text" +) + +// The foreground hook tells the control holding the keyboard that the window +// is coming back, and leaves a control that cannot be told alone. +// +// The other half of TestTheWindowComingBackDrawsOnlyTheMarkThatWasThere, +// which drives a control the way the driver does and takes the hook on +// trust. An outside review of the pull request asked for the hook itself +// to be under a guard, and it was right: the hook lived inside Run, behind +// cgo, where no guard reaches, so a registration taken out or a type guard +// widened would have gone unnoticed until somebody opened the window and +// saw the first menu blue again. gui.WindowReturning is the hook's body in +// a file of its own, and this asks it two things with a canvas of the test +// driver's: a focused menu told the window is returning draws no mark on +// the FocusGained that follows, and a focused box to type in - which is not +// Returnable, and whose focused look is the toolkit's own - is left alone +// rather than made to panic. Nothing focused is the third case. +func TestTheForegroundHookTellsTheFocusedControlTheWindowIsBack(t *testing.T) { + c, content := screenOnACanvas(t) + + menu := chooserUnder(t, content, text.FieldFormat()) + parts.FocusQuietly(c, menu) + if c.Focused() != fyne.Focusable(menu) || menu.Marked() { + t.Fatal("the menu does not hold the keyboard quietly, so this guard is not starting from the state it is about") + } + menu.FocusLost() + gui.WindowReturning(c) + menu.FocusGained() + if menu.Marked() { + t.Error("the hook ran and the focused menu still drew the keyboard mark on the window's return - the hook told it nothing") + } + + // A box to type in cannot be told, and is not to be broken by the + // telling. Its own FocusGained is the toolkit's and draws the box's + // focused edge, which is right: the caret comes back with the window. + box := entryUnder(t, content, text.FieldSize()) + if box == nil { + t.Fatal("there is no size box, so this guard read the wrong tree") + } + c.Focus(box) + if c.Focused() != fyne.Focusable(box) { + t.Fatal("the size box did not take the keyboard, so the second half of this guard cannot start") + } + gui.WindowReturning(c) + + // And nothing at all holding the keyboard. + c.Unfocus() + gui.WindowReturning(c) + gui.WindowReturning(nil) +} + +// And the real window registers exactly that, in the toolkit's foreground +// hook, once - read out of the source, because the registration is behind +// cgo where no guard runs. A registration taken out, or a hook registered +// with some other body, is a window whose first menu opens blue again. +func TestTheWindowBinaryRegistersTheForegroundHook(t *testing.T) { + root := repoRoot(t) + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, filepath.Join(root, "internal", "gui", "run_cgo.go"), nil, 0) + if err != nil { + t.Fatalf("parsing run_cgo.go: %v", err) + } + registered, telling := 0, 0 + ast.Inspect(file, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + if fun, ok := call.Fun.(*ast.SelectorExpr); ok && fun.Sel.Name == "SetOnEnteredForeground" { + registered++ + // The body of what is registered has to call WindowReturning, + // so the hook and the guarded function are one thing. + for _, arg := range call.Args { + ast.Inspect(arg, func(inner ast.Node) bool { + if c, ok := inner.(*ast.CallExpr); ok { + if id, ok := c.Fun.(*ast.Ident); ok && id.Name == "WindowReturning" { + telling++ + } + } + return true + }) + } + } + return true + }) + if registered != 1 { + t.Errorf("run_cgo.go registers the foreground hook %d time(s), and the window binary registers it exactly once", registered) + } + if telling != 1 { + t.Errorf("the registered foreground hook calls WindowReturning %d time(s), and it is the one thing the hook is for", telling) + } +} + +// No file of the window package outside the cgo build reaches the toolkit's +// widgets, so the window binary still compiles where there is no C compiler +// - on every system, darwin included. +// +// Measured on 2026-09-21, the hard way: the first version of returning.go +// imported parts for one interface, and parts reaches the toolkit's widget +// tree, whose internal/widget does not compile on darwin with cgo off +// (ci.yml records it at the matrix, from 2026-08-20). The build guard +// beside this one builds with cgo off on the system it runs on, so Linux +// and Windows stayed green and the macOS job went red. This asks the +// question the same on every system: the import lists of the files that +// take part in the no-cgo build, read from the source with their build +// constraints. A file behind a cgo constraint may import anything. +func TestTheWindowPackageOutsideCgoReachesNoWidget(t *testing.T) { + root := repoRoot(t) + fset := token.NewFileSet() + dir := filepath.Join(root, "internal", "gui") + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("reading internal/gui: %v", err) + } + checked := 0 + for _, e := range entries { + name := e.Name() + if e.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + continue + } + file, perr := parser.ParseFile(fset, filepath.Join(dir, name), nil, parser.ParseComments|parser.ImportsOnly) + if perr != nil { + t.Fatalf("parsing %s: %v", name, perr) + } + if behindCgo(file) { + continue + } + checked++ + for _, imp := range file.Imports { + path := strings.Trim(imp.Path.Value, `"`) + if strings.HasSuffix(path, "/internal/gui/parts") || strings.HasSuffix(path, "/internal/gui/window") || + strings.HasSuffix(path, "/internal/gui/catalogue") || strings.HasPrefix(path, "fyne.io/fyne/v2/widget") || + strings.HasPrefix(path, "fyne.io/fyne/v2/container") { + t.Errorf("internal/gui/%s takes part in the build with cgo off and imports %s, which reaches the toolkit's widgets - "+ + "and those do not compile on darwin without cgo, so the window binary stops building where there is no C compiler", + name, path) + } + } + } + if checked < 3 { + t.Fatalf("only %d files of internal/gui are outside the cgo build, which is fewer than there are - this guard is reading the wrong directory", checked) + } +} + +// behindCgo says whether a file's build constraint keeps it out of the build +// with cgo off. Only the plain "cgo" constraint is looked for, because that +// is the one the window package uses (run_cgo.go). A constraint written any +// other way counts as taking part, which errs towards asking. +func behindCgo(file *ast.File) bool { + for _, group := range file.Comments { + for _, c := range group.List { + if strings.HasPrefix(c.Text, "//go:build") && strings.Contains(c.Text, "cgo") && !strings.Contains(c.Text, "!cgo") { + return true + } + } + } + return false +} diff --git a/internal/gui/catalogue/catalogue.go b/internal/gui/catalogue/catalogue.go index dee9343..eb276a2 100644 --- a/internal/gui/catalogue/catalogue.go +++ b/internal/gui/catalogue/catalogue.go @@ -96,6 +96,7 @@ func NotDrawn() []Reason { {"Tab", "data: one screen and the word that leads to it, drawn by Tabs"}, {"Look", "an enum: which face a Button wears, every value drawn under Button"}, {"PointerFocus", "a piece inside a control, knowing what put the keyboard there - no picture of its own"}, + {"Returnable", "an interface: a control the window can tell it is coming back to the front, nothing of its own to draw"}, {"Shortcuts", "a keyboard map: nothing to draw, and saying so is the point of this row"}, } } @@ -111,6 +112,7 @@ func LayoutOnly() []Reason { {"Indented", "the same left edge as the fields inside a panel"}, {"Sized", "one width, given"}, {"Numeric", "the width a number needs"}, + {"Text", "the width a short piece of text needs - a name, a template, a file name"}, {"Padded", "one distance of the scale round its content"}, {"Stacked", "panels one under another"}, {"Screen", "the readable width round a head and its sections"}, diff --git a/internal/gui/parts/button.go b/internal/gui/parts/button.go index a18ec91..02eb019 100644 --- a/internal/gui/parts/button.go +++ b/internal/gui/parts/button.go @@ -18,8 +18,9 @@ const ( // filled face in the accent colour, so the eye lands on it. Primary Look = iota // Secondary is a button beside the primary one - Preview, Choose, Add a - // batch. An outline round nothing, so it reads as a button without - // competing with the filled one. + // batch. A raised face in the surface a field has, with an edge, so it + // reads as a button without competing with the one in the accent colour. + // It was an outline round nothing until 2026-09-21 - see buttonFace. Secondary // Quiet is a button that is not about the work in front of you - Donate. // Words with a surface only under the pointer, an outline round @@ -403,8 +404,24 @@ func buttonFace(look Look, state buttonState) face { } return f case Secondary: + // A filled face since 2026-09-21, on the owner's report from the + // running window: an outline round nothing, with bold words in it, + // read as a bordered word rather than as something to press - + // Duplicate, Choose, Preview, Add a batch, all of them. The fill is + // the surface a box to type in stands on, with the same edge, and + // that is deliberate rather than a shortcut: on the desktop this + // runs on a button and a field share a surface and are told apart by + // their shape, centred bold words against a value at the left. The + // note on Menu about a control you press drawn as one you type in + // was about a MENU, whose word sits at the left exactly as a field's + // does. The pointer lifts the face and a press lifts it further, the + // way the palette lightens every dark face (ColorNameHover and + // ColorNamePressed), worked out here as one opaque colour each. f := face{ink: theme.ColorNameForeground, edge: PaletteColour(theme.ColorNameInputBorder, dark), edgeWidth: edgeWidth} - f.fill = pointerFill(state) + f.fill = PaletteColour(theme.ColorNameInputBackground, dark) + if wash := pointerFill(state); wash != color.Transparent { + f.fill = blended(f.fill, wash) + } return f default: // Quiet and Glyph: no resting edge, a surface only under the pointer. ink := theme.ColorNameForeground diff --git a/internal/gui/parts/detail.go b/internal/gui/parts/detail.go index 822009c..51bbff9 100644 --- a/internal/gui/parts/detail.go +++ b/internal/gui/parts/detail.go @@ -154,6 +154,11 @@ func newDetailButton(detail Detail) *DetailButton { // drawn is this type's business. func (b *DetailButton) Explanation() string { return b.detail.Text } +// Shown is the box this button has put on the sheet, or nil while there is +// none - for a guard measuring what the explanation is drawn ON, which the +// words alone cannot say. +func (b *DetailButton) Shown() fyne.CanvasObject { return b.shown } + // MouseIn shows the explanation when the pointer arrives. func (b *DetailButton) MouseIn(e *desktop.MouseEvent) { b.Button.MouseIn(e) @@ -211,7 +216,9 @@ func (t *Tips) open(near fyne.CanvasObject, detail string) fyne.CanvasObject { } driver := app.Driver() - box := container.NewStack(panelSurface(), Padded(Inset, Prose(detail))) + // On the surface an open list floats on, not on a panel's - see + // floatingSurface for the report that moved it there. + box := container.NewStack(floatingSurface(), Padded(Inset, Prose(detail))) // Sized twice, and this is the same finding the render probe records rather // than superstition. A wrapping label reports the height it needs for the diff --git a/internal/gui/parts/focus.go b/internal/gui/parts/focus.go index 809e0b0..2a66938 100644 --- a/internal/gui/parts/focus.go +++ b/internal/gui/parts/focus.go @@ -23,7 +23,15 @@ import ( // control has to get the same answer without anybody remembering. See // docs/UX.md section 7.0 gate 2: a fix that has to be repeated is not a fix of // the class. -type PointerFocus struct{ silent bool } +type PointerFocus struct { + silent bool + // returning is set by the window just before the toolkit hands the + // keyboard back to the control that held it, because the WINDOW came to + // the front again - see WindowReturning. wasMarked is whether the mark + // was drawn when the keyboard last left, which is what a return draws. + returning bool + wasMarked bool +} // Quietly moves the focus the way a press does: the control gets the keyboard // and nothing is drawn to say so. @@ -33,8 +41,43 @@ func (p *PointerFocus) Quietly(focus func()) { focus() } -// Quiet reports whether the focus arriving right now came from the pointer. -func (p *PointerFocus) Quiet() bool { return p.silent } +// Draws reports whether the focus arriving right now is to be drawn: not +// when the pointer put it here, and - when the window is coming back to the +// front - only if it was drawn when the window went behind. +// +// The second half is the defect the owner reported on 2026-09-21 as one +// menu wearing a different colour from the others. Measured in the pinned +// toolkit: when the system gives the window the front, the driver calls +// FocusGained on whatever holds the keyboard as if the keyboard had just +// arrived (internal/driver/glfw/window.go processFocused, then +// internal/app/focus_manager.go FocusGained) - and it does so on the very +// first activation, right after the window has put the keyboard on the first +// field quietly. So the first menu on the first screen opened marked, alone +// among every control in the window, and marked again after every Alt-Tab. +// A control cannot tell that call from the keyboard moving to it, because +// both arrive as one FocusGained with the same state - only the window can, +// and it says so through WindowReturning before the call lands. +func (p *PointerFocus) Draws() bool { + if p.returning { + p.returning = false + return p.wasMarked + } + return !p.silent +} + +// Lost is what a control says as the keyboard leaves it: whether its mark +// was drawn. The window coming back draws exactly that again. +func (p *PointerFocus) Lost(marked bool) { p.wasMarked = marked } + +// WindowReturning tells the control that the next FocusGained is the window +// coming back to the front, not the keyboard moving. The real window says it +// from the toolkit's foreground hook, which runs just before the driver's +// call - see Draws, and the wiring in internal/gui. +func (p *PointerFocus) WindowReturning() { p.returning = true } + +// Returnable is a control that can be told the window is coming back, so +// that it does not mistake the toolkit's call for the keyboard arriving. +type Returnable interface{ WindowReturning() } // Take is what a press does with the keyboard: it moves the focus to the // control quietly, unless the control holds it already. diff --git a/internal/gui/parts/foldhead.go b/internal/gui/parts/foldhead.go index d016512..cdda4a0 100644 --- a/internal/gui/parts/foldhead.go +++ b/internal/gui/parts/foldhead.go @@ -90,7 +90,7 @@ func (h *FoldHead) MouseOut() { // FocusGained draws the mark only for the keyboard. See PointerFocus. func (h *FoldHead) FocusGained() { - if h.from.Quiet() { + if !h.from.Draws() { return } h.marked = true @@ -98,10 +98,15 @@ func (h *FoldHead) FocusGained() { } func (h *FoldHead) FocusLost() { + h.from.Lost(h.marked) h.marked = false h.Refresh() } +// WindowReturning is the window saying the next FocusGained is its own +// return to the front. See PointerFocus. +func (h *FoldHead) WindowReturning() { h.from.WindowReturning() } + // TypedRune answers nothing, for the reason Button gives: one press of the // space bar reaches a focused control twice from the desktop driver, as the // key and as the character, and a row answering both would open and shut. diff --git a/internal/gui/parts/listrow.go b/internal/gui/parts/listrow.go index 807929f..363eebd 100644 --- a/internal/gui/parts/listrow.go +++ b/internal/gui/parts/listrow.go @@ -102,26 +102,37 @@ func (r *listRowRenderer) Layout(size fyne.Size) { r.back.Resize(size) r.tick.Resize(fyne.NewSquareSize(icon)) - // The tick stands at the far end of the row, and the words start at the - // gutter - where the word in the box above the list starts. Until + // Two shapes of row, decided by whether the list draws pictures, and both + // are the owner's, from the running window. + // + // A row WITHOUT a picture puts its words at the gutter - where the word + // in the box above the list starts - and the tick at the far end. Until // 2026-09-16 the tick was in front, and its column was kept whether or // not anything in the list was ticked, so the words of every list stood a - // column to the right of the word in the box. On a list of formats the - // picture in front made that look intended, and on a list with no picture - // and nothing chosen it read as words floating in a rectangle - the - // owner's report from the running window (O220). The column is still - // kept, on the right, so a row does not change width when its value is - // chosen. - r.tick.Move(fyne.NewPos(size.Width-rowGutter-icon, (size.Height-icon)/2)) - left, right := float32(rowGutter), float32(rowGutter+icon+rowGap) - - // The kind sits in front of the words, and takes no room at all where - // there is none - so a list of paper sizes is drawn exactly as it was. + // column to the right of the word in the box, and a list with no picture + // and nothing chosen read as words floating in a rectangle (O220). + // + // A row WITH a picture keeps the tick in front, then the picture, then + // the words - the shape the list of formats had before that day, which is + // the shape the owner had said looked right. Moving its tick to the end + // with the others pulled the picture and the word a column to the left, + // and the report of 2026-09-21 was that the list had been broken: what + // stood in the middle of the box now hugged its edge. The column in front + // makes the picture and the word sit where they did, and the tick fills + // it or leaves it empty without the row changing width. + // + // Either way the row is the same width for a chosen value as for any + // other, because the tick's column is kept in both shapes. + left, right := float32(rowGutter), float32(rowGutter) if r.row.kind != nil { + r.tick.Move(fyne.NewPos(left, (size.Height-icon)/2)) + left += icon + rowGap r.kind.Resize(fyne.NewSquareSize(icon)) r.kind.Move(fyne.NewPos(left, (size.Height-icon)/2)) left += icon + rowGap } else { + r.tick.Move(fyne.NewPos(size.Width-rowGutter-icon, (size.Height-icon)/2)) + right += icon + rowGap r.kind.Resize(fyne.NewSquareSize(0)) } diff --git a/internal/gui/parts/openlist.go b/internal/gui/parts/openlist.go index faf25f7..ff69894 100644 --- a/internal/gui/parts/openlist.go +++ b/internal/gui/parts/openlist.go @@ -6,7 +6,6 @@ import ( "strings" "fyne.io/fyne/v2" - "fyne.io/fyne/v2/canvas" "fyne.io/fyne/v2/container" "fyne.io/fyne/v2/theme" "fyne.io/fyne/v2/widget" @@ -224,9 +223,7 @@ func (l *OpenList) CreateRenderer() fyne.WidgetRenderer { // The surface is drawn here rather than left to the popup, so that the // colour a guard measures for "an open list is told from the form behind // it" is the colour actually on the screen. - back := canvas.NewRectangle(Theme().Color(theme.ColorNameMenuBackground, theme.VariantDark)) - back.CornerRadius = RadiusField - return widget.NewSimpleRenderer(container.NewStack(back, container.NewThemeOverride(l.list, rowTheme{}))) + return widget.NewSimpleRenderer(container.NewStack(floatingSurface(), container.NewThemeOverride(l.list, rowTheme{}))) } // rowTheme is our theme with the room between rows taken out. diff --git a/internal/gui/parts/parts.go b/internal/gui/parts/parts.go index 9bcbb82..7b74663 100644 --- a/internal/gui/parts/parts.go +++ b/internal/gui/parts/parts.go @@ -251,6 +251,24 @@ func panelSurface() *canvas.Rectangle { return rect } +// floatingSurface is what anything drawn OVER the form stands on: the list a +// menu drops down, and the explanation behind a field's button. +// +// One function for both since 2026-09-21, and the second of them is why. The +// explanation stood on panelSurface until then, and it opens over a section - +// so a box the colour of the thing under it had no edge anywhere, and the +// owner's report from the running window was a sentence laid straight over +// the form, covering the row beneath. The list had already met the same +// question on 2026-08-12 and the palette answers it: the surface that floats +// is the lightest one, told from a panel by 13.6 L* with no border and no +// shadow (theme.go, ColorNameMenuBackground). The corner is a field's, not a +// panel's, because what floats is the size of a control and not of a section. +func floatingSurface() *canvas.Rectangle { + rect := canvas.NewRectangle(PaletteColour(theme.ColorNameMenuBackground, theme.VariantDark)) + rect.CornerRadius = RadiusField + return rect +} + // Bullets is a list of short statements, drawn as a list. // // It replaces a run of labels each starting with a dash typed into the string. @@ -494,8 +512,9 @@ func (i indent) Layout(objects []fyne.CanvasObject, size fyne.Size) { // Numeric sizes a control to what it holds rather than to the column it is in. // -// Only for boxes taking a number. A path, a name template and an id are all -// things whose length nobody can predict, so those still take the column. +// Only for boxes taking a number. A name, a template and an id are short text +// and take TextWidth (see Text). A path is the one thing whose length nobody +// can predict, so a path still takes the column. // // It uses a layout of ours rather than the toolkit's grid wrap, and the reason // is the edge that marks a refused box: a stack sized by the slot draws its @@ -508,6 +527,13 @@ func Numeric(control fyne.CanvasObject) fyne.CanvasObject { return Sized(NumericWidth, control) } +// Text sizes a box for a short piece of text - a name, a template, a file +// name - to TextWidth rather than to the column it is in. A path is not +// this case and takes the row, see the token. +func Text(control fyne.CanvasObject) fyne.CanvasObject { + return Sized(TextWidth, control) +} + // Sized draws a control at a width worked out by the caller. // // Numeric is the common case and this is the one behind it, for the fields diff --git a/internal/gui/parts/property.go b/internal/gui/parts/property.go index 6f4ce3a..60fd1e2 100644 --- a/internal/gui/parts/property.go +++ b/internal/gui/parts/property.go @@ -253,18 +253,24 @@ func DeclaredFields(declared []format.Property, into *Fields, tips *Tips) ([]Pro // was 140. The declaration says which of the two a setting is, so nothing here // names a format. // -// Only numbers and sizes. A closed set is as wide as its longest value plus -// the arrow, and free text has no length to promise. -// The width is the wider of the number box every other whole number on these +// Numbers and sizes take the number box, free text takes the text box, and a +// closed set is as wide as its longest value plus the arrow (see Menu). Free +// text took the whole row until 2026-09-21 on the sentence that it has no +// length to promise - an archive's password and a preset's list of sizes +// were 806 px wide - and the owner asked why. See TextWidth. +// The width is the wider of the box every other value of that shape on these // screens uses and whatever it takes to show this field's own placeholder. // Shrinking to the first alone clipped "worked out from the size" mid-word - // which is the same defect the other way up, since a box has to be able to // show what it is already showing. func ShapedFor(p format.Property, control fyne.CanvasObject) fyne.CanvasObject { - if !narrowOnAScreen(p) { - return control + switch { + case narrowOnAScreen(p): + return Sized(fyne.Max(NumericWidth, roomFor(leftAlone(p))), control) + case p.Kind == format.PropertyText: + return Sized(fyne.Max(TextWidth, roomFor(leftAlone(p))), control) } - return Sized(fyne.Max(NumericWidth, roomFor(leftAlone(p))), control) + return control } // roomFor is how wide a box has to be to show a string without cutting it. diff --git a/internal/gui/parts/ring.go b/internal/gui/parts/ring.go index 6faf0b8..512a846 100644 --- a/internal/gui/parts/ring.go +++ b/internal/gui/parts/ring.go @@ -2,6 +2,7 @@ package parts import ( "image/color" + "math" "strings" "fyne.io/fyne/v2" @@ -355,9 +356,10 @@ func (c *Chooser) useRing(r *Ring) { // 2026-08-18: a menu stays painted blue after a value is chosen, and there is // nothing left to press that would take the paint off. // -// See PointerFocus for why this is one rule rather than a fix per control. +// See PointerFocus for why this is one rule rather than a fix per control, +// and PointerFocus.Draws for the window coming back to the front. func (c *Chooser) FocusGained() { - if c.from.Quiet() { + if !c.from.Draws() { return } c.mark() @@ -374,6 +376,7 @@ func (c *Chooser) mark() { } func (c *Chooser) FocusLost() { + c.from.Lost(c.marked) c.marked = false c.Select.FocusLost() if c.ring != nil { @@ -381,6 +384,10 @@ func (c *Chooser) FocusLost() { } } +// WindowReturning is the window saying the next FocusGained is its own +// return to the front. See PointerFocus. +func (c *Chooser) WindowReturning() { c.from.WindowReturning() } + // Marked says whether the keyboard mark is drawn, for a guard. The toolkit // keeps the same answer in an unexported field of two different widgets, so // reading it off the canvas means reading a colour and deciding what it meant. @@ -439,7 +446,7 @@ func (c *Chooser) drop(surface fyne.Canvas) { // As wide as the box, so the list reads as belonging to that field. How // tall and which side of the box it goes on is worked out from the room // that is actually left - see roomForList. - height, top := roomForList(surface.Size().Height, at.Y, c.Size().Height, list.MinSize().Height) + height, top := RoomForList(surface.Size().Height, at.Y, c.Size().Height, list.MinSize().Height) // Told to the list rather than only to the popup, because a popup is never // laid out smaller than its content's minimum - so resizing alone left the // list its full height and the shortening did nothing. @@ -453,7 +460,7 @@ func (c *Chooser) drop(surface fyne.Canvas) { list.StartOn(c.Selected) } -// roomForList decides how tall an open list may be and where its top goes. +// RoomForList decides how tall an open list may be and where its top goes. // // It used to go under the box at its full height, always, which is right until // the box is near the foot of a form - and every form here is taller than its @@ -465,40 +472,47 @@ func (c *Chooser) drop(surface fyne.Canvas) { // the window and no more (ListCeiling) - the ceiling is about not taking the // form away from the person reading it, and it follows the window rather than // being a count of rows, which was eight in every window until 2026-09-15 -// (O203). It opens UPWARD when there is more room above the box than below -// it, which is what every desktop menu does. And it is cut to the room on -// whichever side it lands, because the ceiling says nothing about a window -// that has less than that left beside the box. +// (O203). It opens DOWNWARD whenever a few whole rows fit under the box +// (listOpensDownwardFrom), cut to that room and scrolling - and upward only +// when fewer fit there and more fit above, which is a box standing just over +// the bar at the foot. Until 2026-09-21 it turned upward as soon as the list +// did not fit below and there was more room above, which is what a desktop +// menu does and what the owner saw as one list behaving two ways: the format +// list on the preset screen opened over the question the preset asks, and the +// same list on the other screens opened under its box. And it is cut to the +// room on whichever side it lands, in whole rows, because the ceiling says +// nothing about a window that has less than that left beside the box. +// +// Whole rows, so that a list ending short of its ceiling ends on a row's edge +// rather than through the middle of a value - the cut is the everyday case +// now, not the emergency it was when only a cramped window reached it. // -// Arithmetic rather than widgets so that it can be checked directly. The screen -// level guard opens a real menu and measures the overlay, which is the half -// that catches this being wired up wrongly. -func roomForList(canvasHeight, boxTop, boxHeight, wanted float32) (height, top float32) { +// Arithmetic rather than widgets so that it can be checked directly, and +// exported for that check. The screen level guard opens a real menu and +// measures the overlay, which is the half that catches this being wired up +// wrongly. +func RoomForList(canvasHeight, boxTop, boxHeight, wanted float32) (height, top float32) { if ceiling := ListCeiling(canvasHeight); wanted > ceiling { wanted = ceiling } - below := canvasHeight - (boxTop + boxHeight) - listEdgeGap - above := boxTop - listEdgeGap - if below < 0 { - below = 0 - } - if above < 0 { - above = 0 - } + below := wholeRows(canvasHeight - (boxTop + boxHeight) - listEdgeGap) + above := wholeRows(boxTop - listEdgeGap) - if wanted <= below { - return wanted, boxTop + boxHeight + if wanted <= below || below >= listOpensDownwardFrom*listRowHeight() || below >= above { + return fyne.Min(wanted, below), boxTop + boxHeight } - if above > below { - if wanted > above { - wanted = above - } - return wanted, boxTop - wanted - } - if wanted > below { - wanted = below + height = fyne.Min(wanted, above) + return height, boxTop - height +} + +// wholeRows is as much of a height as whole rows of a list fill, and never +// less than nothing. +func wholeRows(height float32) float32 { + row := listRowHeight() + if height < row { + return 0 } - return wanted, boxTop + boxHeight + return float32(math.Floor(float64(height/row))) * row } // giveBack hands the keyboard back to the box when the list closes. diff --git a/internal/gui/parts/segments.go b/internal/gui/parts/segments.go index 505f11f..15eefb0 100644 --- a/internal/gui/parts/segments.go +++ b/internal/gui/parts/segments.go @@ -144,7 +144,7 @@ func (s *Segments) MouseOut() { // FocusGained draws the ring when the keyboard is what brought the focus here. // A press brings it quietly and the first key turns the ring on. func (s *Segments) FocusGained() { - if s.from.Quiet() { + if !s.from.Draws() { return } s.mark() @@ -155,10 +155,15 @@ func (s *Segments) mark() { s.Refresh() } func (s *Segments) FocusLost() { + s.from.Lost(s.marked) s.marked = false s.Refresh() } +// WindowReturning is the window saying the next FocusGained is its own +// return to the front. See PointerFocus. +func (s *Segments) WindowReturning() { s.from.WindowReturning() } + func (s *Segments) TypedRune(rune) {} // TypedKey moves the choice. Left and right step, Home and End jump - and each diff --git a/internal/gui/parts/tabs.go b/internal/gui/parts/tabs.go index 4175958..a2e23c5 100644 --- a/internal/gui/parts/tabs.go +++ b/internal/gui/parts/tabs.go @@ -256,7 +256,7 @@ func (w *TabWord) MouseOut() { // FocusGained draws the mark only for the keyboard. See PointerFocus. func (w *TabWord) FocusGained() { - if w.from.Quiet() { + if !w.from.Draws() { return } w.mark() @@ -268,10 +268,15 @@ func (w *TabWord) mark() { } func (w *TabWord) FocusLost() { + w.from.Lost(w.marked) w.marked = false w.Refresh() } +// WindowReturning is the window saying the next FocusGained is its own +// return to the front. See PointerFocus. +func (w *TabWord) WindowReturning() { w.from.WindowReturning() } + // Quietly runs a focus change without drawing the mark. See FocusQuietly. func (w *TabWord) Quietly(focus func()) { w.from.Quietly(focus) } diff --git a/internal/gui/parts/toggle.go b/internal/gui/parts/toggle.go index feed36b..1a0f81a 100644 --- a/internal/gui/parts/toggle.go +++ b/internal/gui/parts/toggle.go @@ -123,7 +123,7 @@ func (t *Toggle) MouseOut() { // the same rule as the Chooser, so a person reaching for the keyboard after a // click sees which control is listening. func (t *Toggle) FocusGained() { - if t.from.Quiet() { + if !t.from.Draws() { return } t.mark() @@ -135,10 +135,15 @@ func (t *Toggle) mark() { } func (t *Toggle) FocusLost() { + t.from.Lost(t.marked) t.marked = false t.Refresh() } +// WindowReturning is the window saying the next FocusGained is its own +// return to the front. See PointerFocus. +func (t *Toggle) WindowReturning() { t.from.WindowReturning() } + // TypedRune answers nothing, for the reason Button.TypedRune gives: the // desktop driver delivers one press of the space bar as the key and as the // character, and a switch answering both flipped twice on one press - back to @@ -205,8 +210,13 @@ func (r *toggleRenderer) Layout(size fyne.Size) { // thing that changes and not round the room a finger needs. r.ring.Resize(square.Add(fyne.NewSquareSize(ringGap * 2))) r.ring.Move(at.Subtract(fyne.NewPos(ringGap, ringGap))) - r.tick.Resize(fyne.NewSquareSize(markSide - GapInline*2)) - r.tick.Move(at.Add(fyne.NewPos(GapInline, GapInline))) + // The tick is drawn on the WHOLE square. It kept a step of room inside + // the square until 2026-09-21, and the owner's report from the running + // window was a mark too small to read as one: the toolkit's glyph fills + // 56 per cent of its own picture, so a picture 12 px wide drew a tick 7 px + // wide in a square of 20. The glyph's own margin is the room it needs. + r.tick.Resize(square) + r.tick.Move(at) } func (r *toggleRenderer) MinSize() fyne.Size { return fyne.NewSquareSize(GlyphButton) } diff --git a/internal/gui/parts/tokens.go b/internal/gui/parts/tokens.go index 7355755..19279f9 100644 --- a/internal/gui/parts/tokens.go +++ b/internal/gui/parts/tokens.go @@ -167,6 +167,17 @@ const ( // number any of these fields accepts - the ceiling on files is seven digits // and the largest size anybody types is eight. NumericWidth = 140 + // TextWidth is as wide as a box holding a short piece of text gets - a + // batch's name, the template its files are named by, the manifest's file + // name, an archive's password, a preset's list of sizes. Two number boxes + // and the gap between them, so a text box ends where a pair of numbers + // standing side by side would, and the column of controls keeps one + // right edge for everything but a path. Until 2026-09-21 these boxes + // took the whole row on the sentence that free text has no length to + // promise, and the owner's report from the running window was the + // obvious one: why are they so long. A path is the one value that can + // be, so a path still takes the row. + TextWidth = NumericWidth*2 + GapColumns // GlyphButton is the side of the small square button that holds one // glyph - the mark beside a field's name that opens its explanation. The // glyph itself is the toolkit's inline icon, 20, and the square keeps two @@ -201,6 +212,16 @@ const ( // gives ten rows at 600 px, eighteen at 1025 and twenty-three at 1300, and // at 600 the list still ends above the buttons at the foot. listShare = 0.5 + // listOpensDownwardFrom is how many whole rows have to fit under a box + // for its list to open downward, cut to that room and scrolling, rather + // than upward into the room above. Decision of the owner, 2026-09-21, + // from the running window: the format list on the preset screen opened + // upward, over the question the preset asks, while the same list on the + // other screens opened downward - one control behaving two ways for a + // reason nobody could see. Upward is kept for the emergency alone, a box + // standing just above the bar at the foot, where fewer rows than this + // would fit. Five rows is enough to read a list and to see it scrolls. + listOpensDownwardFrom = 5 // rowPadding is the room above and below a list row's contents. Ours rather // than the theme's, which is the entire point of that control: the theme's // inner padding is what a box to type in and a button are also built from, diff --git a/internal/gui/returning.go b/internal/gui/returning.go new file mode 100644 index 0000000..18a1d9f --- /dev/null +++ b/internal/gui/returning.go @@ -0,0 +1,44 @@ +package gui + +import "fyne.io/fyne/v2" + +// WindowReturning tells the control holding the keyboard that the window is +// coming back to the front, so that the toolkit's next FocusGained is not +// mistaken for the keyboard arriving. +// +// Measured in the pinned toolkit on 2026-09-21: whenever the system gives +// the window the front, the driver calls FocusGained on whatever holds the +// keyboard as if the keyboard had just moved there +// (internal/driver/glfw/window.go, processFocused) - on the very first +// activation too, right after Open has put the keyboard on the first field +// quietly. So the first menu on the first screen opened marked, alone among +// every control in the window, which the owner reported as one menu wearing +// a different colour from the rest. The foreground hook runs just before +// that call, and this is what the real window registers there - see Run. +// +// Its own file, outside the cgo build, so that a guard can call it with a +// canvas of the test driver's and a focused control of ours. The line that +// registers it is behind cgo like the rest of the real window, and a guard +// reads that line out of the source instead - the same split the refusal +// seam has (OpenOrRefuse and TestTheWindowBinaryOpensThroughTheRefusalSeam). +// +// It asks for the method by shape rather than importing parts.Returnable, +// and that is a constraint of the build and not a style: the files of this +// package outside the cgo build must not reach the toolkit's widgets, +// because on darwin without cgo the toolkit's own internal/widget does not +// compile (ci.yml says so at the matrix, measured 2026-08-20) - and the +// guard that builds the window binary with cgo off runs on every system. +// The first version imported parts for the interface and turned the macOS +// job red on 2026-09-21 for exactly that. +// +// A control that cannot be told - a box to type in, whose focused look is +// the toolkit's own and is right to come back with the window - is left +// alone. Nothing holding the keyboard is left alone too. +func WindowReturning(c fyne.Canvas) { + if c == nil { + return + } + if returning, ok := c.Focused().(interface{ WindowReturning() }); ok { + returning.WindowReturning() + } +} diff --git a/internal/gui/run_cgo.go b/internal/gui/run_cgo.go index d19a428..8d48378 100644 --- a/internal/gui/run_cgo.go +++ b/internal/gui/run_cgo.go @@ -9,6 +9,7 @@ import ( "log" "net/url" "path/filepath" + "time" "fyne.io/fyne/v2" "fyne.io/fyne/v2/app" @@ -86,6 +87,15 @@ type desktop struct { // while the driver draws it would be saying something untrue. func (d desktop) SoftwareRendering() bool { return d.software } +// Later fires on a clock and lands on the interface thread through the +// toolkit's queue, which is what fyne.Do is for. Calling it off stops the +// clock, and a function already on its way finds the state it checks +// before it acts - see busy.set. +func (d desktop) Later(after time.Duration, then func()) func() { + timer := time.AfterFunc(after, func() { fyne.Do(then) }) + return func() { timer.Stop() } +} + // Remembered is the window size and output directory, kept by the toolkit in // the file its folder picker already writes. // @@ -292,6 +302,13 @@ func run(launch Launch, errOut io.Writer) int { a.Settings().SetTheme(parts.Theme()) w := a.NewWindow(text.WindowTitle(version.Version)) host := desktop{Window: w, software: software} + // The window coming to the front is not the keyboard arriving, and only + // the window can tell the two apart - see WindowReturning for the + // measurement. The foreground hook runs just before the driver's call, so + // this is where the control is told what is coming. The registration is + // behind cgo like the remembering above, so a guard reads this line out + // of the source and calls what it registers with a canvas of its own. + a.Lifecycle().SetOnEnteredForeground(func() { WindowReturning(w.Canvas()) }) // What a first start opens at, if nothing is remembered: nothing for the // catalogue, which opens at the ceiling, and what the screens want for // the ordinary window. diff --git a/internal/gui/window/generate.go b/internal/gui/window/generate.go index 8d8157c..b016799 100644 --- a/internal/gui/window/generate.go +++ b/internal/gui/window/generate.go @@ -2,6 +2,7 @@ package window import ( "errors" + "time" "fyne.io/fyne/v2" "fyne.io/fyne/v2/container" @@ -96,6 +97,19 @@ type Host interface { // was going to go. OpenLink(url string) + // Later runs something on the interface thread after a while, and hands + // back the way to call it off. What the busy face waits on - see + // BusyFaceAfter. + // + // On the interface rather than a timer in the screens, and it is the + // same reason as Canvas: only a real window owns the thread that draws, + // and only it can promise that a function fired by a clock lands there. + // A stand in keeps the function and fires it when the guard says, which + // is what lets a guard see the screen BEFORE the face arrives - under the + // test driver a timer's goroutine would be a second writer to widgets + // the guard is reading, and the race detector would be right. + Later(after time.Duration, then func()) (callOff func()) + // SoftwareRendering reports whether this window is drawn by the software // renderer shipped beside the program rather than by the graphics driver. // @@ -172,7 +186,7 @@ type Generate struct { // NewGenerate builds the screen. links are the buttons to the other screens. func NewGenerate(host Host, links ...fyne.CanvasObject) *Generate { - g := &Generate{runner: newRunner(), host: host, tips: parts.NewTips(), settingsFolded: true} + g := &Generate{runner: newRunner(host.Later), host: host, tips: parts.NewTips(), settingsFolded: true} g.runner.settle = g.settle g.runner.openFolder = host.OpenFolder // This screen is one target and draws its boxes under the bare key, so a @@ -238,7 +252,8 @@ func NewGenerate(host Host, links ...fyne.CanvasObject) *Generate { func (g *Generate) Object() fyne.CanvasObject { return g.body } // Unscrolled is how tall this screen has to be for its form to show whole - -// what a first start opens at, see firstOpening. +// what a first start opens at, because this is the screen it opens on. See +// firstOpening for why the other two screens are not asked. func (g *Generate) Unscrolled() float32 { return unscrolledHeight(g.body, g.scroll) } // FirstField is where the keyboard starts on this screen: the format, because @@ -351,9 +366,9 @@ func (g *Generate) settingsSection() []fyne.CanvasObject { add(format.SettingSize, text.FieldSize(), text.HintSize(), g.tips.Say(text.DetailSize()), parts.Numeric(g.size)), add(engine.SettingCount, text.FieldCount(), "", parts.NoDetail, parts.Numeric(g.count)), - add(engine.SettingID, text.FieldTargetID(), text.HintTargetID(), g.tips.Say(text.DetailTargetID()), g.id), + add(engine.SettingID, text.FieldTargetID(), text.HintTargetID(), g.tips.Say(text.DetailTargetID()), parts.Text(g.id)), add(engine.SettingName, text.FieldNameTemplate(), text.HintNameTemplate(), - g.tips.Say(text.DetailNameTemplate()), g.name), + g.tips.Say(text.DetailNameTemplate()), parts.Text(g.name)), // The settings the chosen format declares land here, under the ones // every format has. g.propBox, diff --git a/internal/gui/window/open.go b/internal/gui/window/open.go index a114996..c155b58 100644 --- a/internal/gui/window/open.go +++ b/internal/gui/window/open.go @@ -134,10 +134,11 @@ func Open(h Host) fyne.Size { // Last, once there is something on the canvas to focus. Quietly: nobody // has pressed a key yet. focusFirst(showing, false) - // What the window has to be for the tallest work screen to show whole, - // worked out from the screens as built rather than measured once and - // typed in - the size a first start opens at, see HowToOpen. - return firstOpening(tabbed, gen, pre, rec) + // What the window has to be for the screen it opens ON to show whole, + // worked out from that screen as built rather than measured once and + // typed in - the size a first start opens at, see HowToOpen and + // firstOpening for why the other two screens are not asked. + return firstOpening(tabbed, gen) } // closeCleanly stops whatever is running, writes down where the files were diff --git a/internal/gui/window/opening.go b/internal/gui/window/opening.go index eae3098..3090923 100644 --- a/internal/gui/window/opening.go +++ b/internal/gui/window/opening.go @@ -58,11 +58,11 @@ var LargestOpening = fyne.NewSize(1000, 1000) // // A first start opens in the middle, because there is nothing to restore and // the middle is where a window belongs when nobody has an opinion yet. It -// opens as tall as the screens WANT - the height at which the tallest work -// screen shows its whole form without scrolling, worked out by Open from the -// screens as they are - and no taller than LargestOpening, which is the one -// fact about somebody else's screen this program knows. A want with a nought -// in it is no want, and the ceiling stands in for it. +// opens as tall as the first screen WANTS - the height at which the screen +// the window opens on shows its whole form without scrolling, worked out by +// Open from that screen as it is - and no taller than LargestOpening, which +// is the one fact about somebody else's screen this program knows. A want +// with a nought in it is no want, and the ceiling stands in for it. func HowToOpen(remembered, wanted fyne.Size) (size fyne.Size, centre bool) { if WorthRemembering(remembered) { return remembered, false @@ -79,12 +79,23 @@ func HowToOpen(remembered, wanted fyne.Size) (size fyne.Size, centre bool) { type unscrolled interface{ Unscrolled() float32 } // firstOpening is the size the window wants on a first start: the room the -// tab strip keeps above a screen, plus the tallest of the work screens shown +// tab strip keeps above a screen, plus the screen the window opens on shown // whole, at the width the window always opens at. // -// The About screen is left out on purpose. It carries the notices for every -// module in the build, which is a length that belongs to the dependencies and -// not to us, and a window sized for it would be sized for nothing else. +// The screen it opens on, and not the tallest of the three - the owner's +// decision of 2026-09-21. Until then the height came from the tallest work +// screen, the batch screen, so that no work screen scrolled from the first +// frame. What that bought was a window that opened on the single batch +// screen with a band of nothing between its form and the bar - 70 px in the +// forms of that day, the same band O202 was about under another number - +// because the screen a person actually sees first is the shortest. Now the +// window fits what it shows, and the batch screen scrolls a little when +// somebody goes there, which it does anyway from the second batch on. +// +// The About screen is left out for the same reason it always was. It carries +// the notices for every module in the build, which is a length that belongs +// to the dependencies and not to us, and a window sized for it would be sized +// for nothing else. // // The content is laid out at the opening width before anything is measured, // because a sentence that wraps reports a height for the width it currently @@ -97,13 +108,9 @@ type unscrolled interface{ Unscrolled() float32 } // padded by default and never switched off here. Measured before it was added: // the batch screen's form needed 732 px and got 724 at the size worked out // without it, eight pixels of padding short of showing whole. -func firstOpening(tabbed *fyne.Container, screens ...unscrolled) fyne.Size { +func firstOpening(tabbed *fyne.Container, first unscrolled) fyne.Size { tabbed.Resize(LargestOpening) - tallest := float32(0) - for _, s := range screens { - tallest = fyne.Max(tallest, s.Unscrolled()) - } - return fyne.NewSize(LargestOpening.Width, parts.AboveTheScreens(tabbed)+tallest+2*theme.Padding()) + return fyne.NewSize(LargestOpening.Width, parts.AboveTheScreens(tabbed)+first.Unscrolled()+2*theme.Padding()) } // unscrolledHeight is how tall a screen has to be for the form inside its diff --git a/internal/gui/window/preset.go b/internal/gui/window/preset.go index 39b7534..6a6c4de 100644 --- a/internal/gui/window/preset.go +++ b/internal/gui/window/preset.go @@ -52,7 +52,7 @@ type Preset struct { // NewPreset builds the screen. links are the buttons to the other screens. func NewPreset(host Host, links ...fyne.CanvasObject) *Preset { - p := &Preset{runner: newRunner(), host: host, tips: parts.NewTips()} + p := &Preset{runner: newRunner(host.Later), host: host, tips: parts.NewTips()} p.runner.openFolder = host.OpenFolder p.runner.settle = p.settle // No readdress here, and that is the boundary of this screen rather than an @@ -140,10 +140,6 @@ const settingPreset = "preset" // Object is the screen, to put in a window. func (p *Preset) Object() fyne.CanvasObject { return p.body } -// Unscrolled is how tall this screen has to be for its form to show whole - -// what a first start opens at, see firstOpening. -func (p *Preset) Unscrolled() float32 { return unscrolledHeight(p.body, p.scroll) } - // FirstField is where the keyboard starts: which preset, because every // parameter under it is drawn from that answer. func (p *Preset) FirstField() fyne.Focusable { return p.pick } diff --git a/internal/gui/window/recipe.go b/internal/gui/window/recipe.go index 2f41257..2690266 100644 --- a/internal/gui/window/recipe.go +++ b/internal/gui/window/recipe.go @@ -154,7 +154,7 @@ type content struct { // anybody does a press of "Add a batch" to find out what the screen is. One is // also what the single batch screen shows, so the two read as one tool. func NewRecipe(host Host, links ...fyne.CanvasObject) *Recipe { - r := &Recipe{runner: newRunner(), host: host, tips: parts.NewTips()} + r := &Recipe{runner: newRunner(host.Later), host: host, tips: parts.NewTips()} r.runner.settle = r.settle r.runner.openFolder = host.OpenFolder // A refusal about a size belongs on the box the switch is showing. @@ -186,7 +186,7 @@ func NewRecipe(host Host, links ...fyne.CanvasObject) *Recipe { // It is disabled with the rest of the form while a run is going, because // adding a batch mid run would rebuild the form under the run. r.addBtn = parts.NewButton(parts.Secondary, text.ButtonAddBatch(), r.addBatch) - r.runner.alsoDisabled = append(r.runner.alsoDisabled, r.addBtn) + r.runner.busy.also = append(r.runner.busy.also, r.addBtn) r.body = r.tips.Over(container.NewBorder( nil, @@ -215,10 +215,6 @@ func NewRecipe(host Host, links ...fyne.CanvasObject) *Recipe { // Object is the screen, to put in the window. func (r *Recipe) Object() fyne.CanvasObject { return r.body } -// Unscrolled is how tall this screen has to be for its form to show whole - -// what a first start opens at, see firstOpening. -func (r *Recipe) Unscrolled() float32 { return unscrolledHeight(r.body, r.scroll) } - // FirstField is where the keyboard starts: the format of the first batch. There // is always a first batch - the last one cannot be removed. func (r *Recipe) FirstField() fyne.Focusable { return r.batches[0].formatPick } @@ -387,7 +383,7 @@ func (r *Recipe) batchBlock(index int, b *batch) fyne.CanvasObject { // nothing further to add. r.tips.Say(""), b.formatPick), add(recipe.KeyID, text.FieldTargetID(), text.HintTargetID(), - r.tips.Say(text.DetailTargetID()), b.id), + r.tips.Say(text.DetailTargetID()), parts.Text(b.id)), add(recipe.KeyCount, text.FieldCount(), "", parts.NoDetail, parts.Numeric(b.count)), // One way of saying how big, chosen from three, since 2026-08-25. // @@ -398,7 +394,7 @@ func (r *Recipe) batchBlock(index int, b *batch) fyne.CanvasObject { // is a full row wide rather than a third of one. r.sizeWayFor(b, at, add), add(recipe.KeyName, text.FieldNameTemplate(), text.HintNameTemplate(), - r.tips.Say(text.DetailNameTemplate()), b.name), + r.tips.Say(text.DetailNameTemplate()), parts.Text(b.name)), ) // A format that declares nothing has no settings section at all, so the @@ -540,7 +536,7 @@ func (r *Recipe) outputSection() fyne.CanvasObject { r.fields.Add(recipe.KeyOutputDir, text.FieldOutputDir(), text.HintOutputDir(), r.tips.Say(text.DetailOutputDir()), chooserFor(r.host, r.outDir)), r.fields.Add(recipe.KeyOutputManifest, text.FieldManifest(), text.HintManifest(), - r.tips.Say(text.DetailManifest()), r.manifest), + r.tips.Say(text.DetailManifest()), parts.Text(r.manifest)), r.fields.Add(recipe.KeySeed, text.FieldSeed(), text.HintSeed(), r.tips.Say(text.DetailSeed()), parts.Numeric(r.seed)), r.fields.AddToggle(recipe.KeyDefaultsLabel, text.FieldLabel(), "", diff --git a/internal/gui/window/recipefolds.go b/internal/gui/window/recipefolds.go index a438dd6..8cb9155 100644 --- a/internal/gui/window/recipefolds.go +++ b/internal/gui/window/recipefolds.go @@ -86,7 +86,7 @@ func (r *Recipe) manifestNotes(b *batch, add addField) fyne.CanvasObject { b.notes = parts.NewInnerFolding(text.SectionManifestNotes(), parts.Note(text.NoteManifestOnly()), add(recipe.KeyGroup, text.FieldGroup(), text.HintGroup(), - r.tips.Say(text.DetailGroup()), b.group), + r.tips.Say(text.DetailGroup()), parts.Text(b.group)), add(recipe.KeyExpected, text.FieldExpected(), text.HintExpected(), r.tips.Say(text.DetailExpected()), b.expected), add(recipe.KeyExpectedReason, text.FieldReason(), text.HintReason(), diff --git a/internal/gui/window/run.go b/internal/gui/window/run.go index 4d2cf1a..26e86d4 100644 --- a/internal/gui/window/run.go +++ b/internal/gui/window/run.go @@ -79,7 +79,6 @@ type runner struct { previewBtn *parts.Button generateBtn *parts.Button - cancelBtn *parts.Button // openBtn shows the directory a finished run wrote into. // // It appears when there is something to open and goes away the moment the @@ -96,7 +95,9 @@ type runner struct { // where the files ACTUALLY went. wroteInto string - bar *parts.Progress + // busy is whether work owns the screen and the face it wears for it - + // the frozen form, Cancel, the bar. See runbusy.go. + busy *busy status *widget.Label problem *parts.ErrorArea @@ -157,23 +158,6 @@ type runner struct { // shared by three screens and none of them owns the window. openFolder func(string) - // running is whether a run owns the screen, and it exists because stop - // cannot answer that. Asking stop was a real defect and a quiet one: it is - // set on the first Generate and never cleared, so from then on every live - // check returned at its first line. Fields stopped being checked while - // being typed in and stopped being unmarked once corrected, and the screen - // looked exactly the same doing it. - // - // Only ever touched on the interface thread, which is what makes a plain - // bool enough - setRunning is called from there, and the worker gets back - // through fyne.Do before it reaches this. - running bool - - // alsoDisabled are controls that are neither fields nor run buttons and - // still have no business being pressed while a run is going. The batch - // screen's "add a batch" is one: pressing it rebuilds the form under a run. - alsoDisabled []fyne.Disableable - // scroll is the part of this screen that moves, so a refusal can bring the // box it is about into view. Set by the screen, because only the screen // that built it knows which scroll holds its form. @@ -324,7 +308,7 @@ func (r *runner) toneOfOutcome(res *engine.Result, runErr error) { // Not while a run owns the screen: its progress is not to be overwritten by // a summary, and the form is frozen then anyway. func (r *runner) refreshLine() { - if r.settle == nil || r.running { + if r.settle == nil || r.busy.occupied { return } dir := "" @@ -339,22 +323,22 @@ func (r *runner) refreshLine() { showOn(r.status, r.line.said(summarise(targets), opt.OutDir)) } -func newRunner() *runner { +func newRunner(wait later) *runner { r := &runner{fields: parts.NewFields(), line: &runLine{}} r.fields.LabelColumn(labelColumn()) // Wired once, here, so that a field added later is covered without anybody // remembering to wire it. See Fields.WhenTypedIn and recheck. r.fields.WhenTypedIn(r.recheck) - r.bar = parts.NewProgress() + bar := parts.NewProgress() // Counted as a percentage rather than as bytes, so the arithmetic that keeps // a very large run inside the range of its own type is the one the command // line already uses. - r.bar.Max = 100 + bar.Max = 100 // Nothing is written inside the track, which is now a property of the // control rather than a formatter turned off: the line under it ends with // the same percentage already (text.Progress), so the number stood on the // screen twice. - r.bar.Hide() + bar.Hide() r.status = widget.NewLabel("") r.status.Wrapping = fyne.TextWrapWord @@ -379,9 +363,11 @@ func newRunner() *runner { // button. The rank it needs is "as pressable as Preview and not competing // with Generate", and Generate is disabled while this one is showing // anyway. - r.cancelBtn = parts.NewButton(parts.Secondary, text.ButtonCancel(), r.onCancel) - r.cancelBtn.Disable() - r.cancelBtn.Hide() + cancel := parts.NewButton(parts.Secondary, text.ButtonCancel(), r.onCancel) + cancel.Disable() + cancel.Hide() + r.busy = &busy{fields: r.fields, preview: r.previewBtn, generate: r.generateBtn, + cancel: cancel, bar: bar, later: wait} r.openBtn = parts.NewButton(parts.Secondary, text.ButtonOpenFolder(), func() { if r.wroteInto != "" && r.openFolder != nil { @@ -412,7 +398,7 @@ func roomToSpeak(bar *parts.Progress, status *widget.Label, problem *parts.Error // footer is the bar at the foot of a screen: the buttons, and under them the // room a run speaks in - which at rest carries what the form comes to. func (r *runner) footer(rail fyne.CanvasObject) fyne.CanvasObject { - return parts.ActionBar(rail, r.actions(), roomToSpeak(r.bar, r.status, r.problem)) + return parts.ActionBar(rail, r.actions(), roomToSpeak(r.busy.bar, r.status, r.problem)) } // onPreview says what the run would cost and writes nothing. @@ -430,6 +416,10 @@ func (r *runner) footer(rail fyne.CanvasObject) fyne.CanvasObject { // busy. Nobody measured how long it takes, which is the point - the answer // depends on somebody else's disk. func (r *runner) onPreview() { + // The same refusal as onGenerate, for the same moment. + if r.busy.occupied { + return + } r.clearProblems() targets, opt, err := r.settle() if err != nil { @@ -452,7 +442,7 @@ func (r *runner) onPreview() { // also meant closing the window waited for the whole of it on the interface // thread. Both halves are gone: preflight checks its context per file, and // planning does too. - r.setBusy(true, true) + r.busy.set(true, busyFace{stoppable: true}) r.say(text.WorkingOutTheCost()) ctx, cancel := context.WithCancel(context.Background()) @@ -520,7 +510,7 @@ func roomOn(dir string) diskRoom { // document - see manifestReachNote - so the answer is there for the asking // rather than something the window would have to work out for itself. func (r *runner) previewFinished(res *engine.Result, planned []engine.PlannedFile, opt engine.Options, room diskRoom, runErr error) { - r.setBusy(false, false) + r.busy.set(false, busyFace{}) if runErr != nil { r.refuse(runErr) return @@ -573,6 +563,13 @@ func formatsOf(planned []engine.PlannedFile) []string { // What is left here is reading the form, which is the one thing that HAS to be // here - the widgets belong to this thread. func (r *runner) onGenerate() { + // Refused by the state and not only by the button. The button is switched + // off with the busy face, which follows the state by a moment - see + // BusyFaceAfter - and a second press inside that moment would start a + // second run into the directory the first is filling. + if r.busy.occupied { + return + } r.clearProblems() targets, opt, err := r.settle() if err != nil { @@ -595,8 +592,8 @@ func (r *runner) startRun(targets []engine.Target, opt engine.Options) { started := time.Now() limit := &throttle{} - r.setRunning(true) - r.bar.SetValue(0) + r.busy.set(true, busyFace{stoppable: true, progressing: true}) + r.busy.bar.SetValue(0) // The plan comes first now, so the first thing said is about working the // cost out rather than about writing files that are not being written yet. r.say(text.WorkingOutTheCost()) @@ -610,7 +607,7 @@ func (r *runner) startRun(targets []engine.Target, opt engine.Options) { } elapsed := time.Since(started) fyne.Do(func() { - r.bar.SetValue(float64(core.Percent(p.BytesDone, p.BytesTotal))) + r.busy.bar.SetValue(float64(core.Percent(p.BytesDone, p.BytesTotal))) r.status.SetText(progressText(p, elapsed)) }) } @@ -640,7 +637,7 @@ func (r *runner) startRun(targets []engine.Target, opt engine.Options) { // run - runFinished would talk about files that never existed. r.holdBeforeFinishing() fyne.Do(func() { - r.setRunning(false) + r.busy.set(false, busyFace{}) r.refuse(planErr) }) close(done) @@ -677,7 +674,7 @@ func (r *runner) startRun(targets []engine.Target, opt engine.Options) { // Note what it does not do: clear stop. That is deliberate and the reason is at // the declaration of the field. func (r *runner) runFinished(res *engine.Result, runErr, saveErr error, room diskRoom) { - r.setRunning(false) + r.busy.set(false, busyFace{}) if room.known && r.wroteInto != "" { r.line.measured(r.wroteInto, room.free) } @@ -705,71 +702,6 @@ func (r *runner) runFinished(res *engine.Result, runErr, saveErr error, room dis r.offerTheFolder(res) } -// setRunning is the screen in one state or the other. Two buttons that both -// look pressable during a run is a window that invites a second run into a -// directory the first one is still filling. -func (r *runner) setRunning(running bool) { r.setBusy(running, running) } - -// setBusy is the same thing with the two halves told apart: whether the screen -// is occupied, and whether there is anything to interrupt. -// -// They came apart when the preview stopped blocking the interface thread. A -// preview occupies the screen exactly as a run does - both buttons off, the -// form frozen - but it has nothing to show on the bar and nothing to cancel, -// and both of those are measured rather than assumed: -// -// - A dry run returns before the writing loop, so OnProgress never fires. A -// bar shown for it would sit at nought until the answer arrived, which is -// what a stuck run looks like. -// - preflight takes no context. It is where a preview spends its time, and it -// cannot be interrupted, so a Cancel offered here would be a button that -// does nothing while looking like the way out. -func (r *runner) setBusy(busy, stoppable bool) { - // The state itself, before any of the controls. Everything below is what - // the state looks like - this is the state, and it is what the live check - // asks. See the running field. - r.running = busy - // Cancel is hidden rather than greyed when there is nothing to cancel, asked - // for on 2026-08-11 after looking at the window. A permanently dead control - // is a question the screen keeps asking and answering itself, and it sat - // beside the two buttons that do work - so the row read as three choices - // where there were two. It appears with the run and goes with it. - // The form goes with them. It stayed editable through a run, so somebody - // could change the output directory while files were going into the old - // one and nothing said which run that applied to - almost certainly none of - // them, which is exactly the answer a person cannot reach from looking - // (O106). - r.fields.Freeze(busy) - for _, control := range r.alsoDisabled { - if busy { - control.Disable() - continue - } - control.Enable() - } - - if busy { - r.previewBtn.Disable() - r.generateBtn.Disable() - } else { - r.previewBtn.Enable() - r.generateBtn.Enable() - } - - // Only a run gets these two. See the note above for why a preview gets - // neither, and note that both are put away whenever the screen goes idle - - // so a preview started after a run cannot leave a stale bar behind. - if busy && stoppable { - r.cancelBtn.Enable() - r.cancelBtn.Show() - r.bar.Show() - return - } - r.cancelBtn.Disable() - r.cancelBtn.Hide() - r.bar.Hide() -} - // keepScroll remembers the scrolling area on the way past, so that a refusal // can bring the box it is about into view. It hands the scroll straight back, // so a screen wires it by wrapping the call it was already making rather than diff --git a/internal/gui/window/runactions.go b/internal/gui/window/runactions.go index 34f2b34..95dd2aa 100644 --- a/internal/gui/window/runactions.go +++ b/internal/gui/window/runactions.go @@ -47,7 +47,7 @@ func (r *runner) PressGenerate() { pressIfLive(r.generateBtn) } func (r *runner) PressPreview() { pressIfLive(r.previewBtn) } -func (r *runner) PressCancel() { pressIfLive(r.cancelBtn) } +func (r *runner) PressCancel() { pressIfLive(r.busy.cancel) } // pressIfLive presses a button somebody could have pressed. // @@ -75,8 +75,12 @@ func (r *runner) actions() fyne.CanvasObject { // Everything that is not one of these buttons went to the bar's rail on // 2026-08-19 - see parts.ActionBar. It used to be laid over this row, which // kept it inside the form's column and so a margin away from the edge. - return container.NewHBox( - layout.NewSpacer(), r.previewBtn, r.generateBtn, r.cancelBtn, r.openBtn, layout.NewSpacer()) + // + // Kept by the busy state as well, because the toolkit does not lay the + // row out again when a button in it is hidden - see busy.relay. + r.busy.row = container.NewHBox( + layout.NewSpacer(), r.previewBtn, r.generateBtn, r.busy.cancel, r.openBtn, layout.NewSpacer()) + return r.busy.row } // offerTheFolder shows the way to the files, once there are some. @@ -92,6 +96,7 @@ func (r *runner) offerTheFolder(res *engine.Result) { return } r.openBtn.Show() + r.busy.relay() } // hideTheFolder takes the offer away when the next run starts, so the button @@ -99,4 +104,5 @@ func (r *runner) offerTheFolder(res *engine.Result) { func (r *runner) hideTheFolder() { r.wroteInto = "" r.openBtn.Hide() + r.busy.relay() } diff --git a/internal/gui/window/runbusy.go b/internal/gui/window/runbusy.go new file mode 100644 index 0000000..7b8df57 --- /dev/null +++ b/internal/gui/window/runbusy.go @@ -0,0 +1,182 @@ +package window + +import ( + "time" + + "fyne.io/fyne/v2" + + "github.com/donislawdev/TestingFilesGenerator/internal/gui/parts" +) + +// The screen while a run or a preview owns it, and the face it wears for that. +// +// Split out of the runner on 2026-09-21, when the face stopped arriving at +// once. The runner stands at its ceiling of fields and of methods, so the +// state and the four controls that show it moved here together rather than +// a timer being squeezed in beside them. + +// BusyFaceAfter is how long a run or a preview may take before the screen +// shows that it is busy - the form frozen, Cancel offered, the bar for a run. +// +// A delay rather than at once, on the owner's report from the running window +// on 2026-09-21 that the whole window shook when Preview was pressed. +// Measured on that window: a preview of one file is done in about 50 ms, and +// in that time the screen froze every box - the toolkit draws a frozen box +// with a bright edge and grey words - put a Cancel button into the row, which +// moved Preview and Generate half a button to the left, showed a bar +// standing at nought, and then took all of it back. A flash, not a state. +// +// Work that is over before anybody could have read the busy face never +// needs one, and work that lasts gets the face in time to matter. The number +// is the one a desktop waits before it changes the pointer to an hourglass +// and the web before it shows a spinner: long enough that a fast answer +// shows nothing at all, short enough that a slow one is not mistaken for a +// button that did nothing. +const BusyFaceAfter = 300 * time.Millisecond + +// later is what a host offers for running something on the interface thread +// after a while: it hands back the way to call it off. See Host.Later. +type later func(after time.Duration, then func()) (callOff func()) + +// busy is whether a run or a preview owns the screen, and what the screen +// wears while one does. +// +// The state and the face are two things and they come apart on purpose. The +// state is set the moment work starts, so that a second press of Generate +// during the first moment of the first is refused - see runner.onGenerate. +// The face follows after BusyFaceAfter, if the work is still going, and comes +// off at once when it ends. +type busy struct { + // occupied is the state: whether work owns the screen. It exists because + // the runner's stop cannot answer that - stop is set on the first press + // and never cleared. Only ever touched on the interface thread, which is + // what makes a plain bool enough. + occupied bool + // worn is whether the face is on, so it is taken off exactly when it + // was put on and never twice. + worn bool + + fields *parts.Fields + preview *parts.Button + generate *parts.Button + cancel *parts.Button + bar *parts.Progress + // also are controls that are neither fields nor run buttons and still + // have no business being pressed while work is going. The batch screen's + // "add a batch" is one: pressing it rebuilds the form under a run. + also []fyne.Disableable + // row is the row the buttons stand in, laid out again whenever one of + // them comes or goes - see relay. + row *fyne.Container + + later later + callOff func() + // epoch counts the pieces of work that have owned the screen, so that a + // face asked for by one of them can never dress the next. Calling the + // clock off is not enough: the real window's clock hands the face to + // the toolkit's queue, and a face already queued when the work ends + // still runs - after the next work has started, if the next press comes + // in that gap. An outside review of the pull request named it. + epoch int +} + +// busyFace is which parts of the face a piece of work earns: whether there +// is anything to stop, and whether there is progress to show. A preview can +// be stopped since 2026-08-26 and reports no progress - a dry run returns +// before the writing loop, so a bar for it would stand at nought for as long +// as the preview took, which is what a stuck run looks like. +type busyFace struct{ stoppable, progressing bool } + +// set puts the screen into the state, and puts the face on or off. The face +// goes on later - BusyFaceAfter from now, on the interface thread, if the +// work is still going - and comes off at once. +func (b *busy) set(occupied bool, face busyFace) { + b.occupied = occupied + if b.callOff != nil { + b.callOff() + b.callOff = nil + } + if !occupied { + b.undress() + return + } + b.epoch++ + mine := b.epoch + b.callOff = b.later(BusyFaceAfter, func() { + // The same work still going, and not already worn: a face asked for + // by earlier work is a face for a screen that has moved on, and a + // face put on twice would be taken off once. + if b.epoch == mine && b.occupied && !b.worn { + b.wear(face) + } + }) +} + +// wear is the face: the form frozen, the buttons that start work off, and +// whatever the work earned on top. +// +// The form goes with the buttons. It stayed editable through a run, so +// somebody could change the output directory while files were going into +// the old one and nothing said which run that applied to - almost certainly +// none of them, which is exactly the answer a person cannot reach from +// looking (O106). +func (b *busy) wear(face busyFace) { + b.worn = true + b.fields.Freeze(true) + for _, control := range b.also { + control.Disable() + } + b.preview.Disable() + b.generate.Disable() + // Cancel is hidden rather than greyed when there is nothing to cancel, + // asked for on 2026-08-11 after looking at the window. A permanently dead + // control is a question the screen keeps asking and answering itself, and + // it sat beside the two buttons that do work - so the row read as three + // choices where there were two. It appears with the work and goes with + // it. + if face.stoppable { + b.cancel.Enable() + b.cancel.Show() + } + if face.progressing { + b.bar.Show() + } + b.relay() +} + +// undress takes the face off, whether or not it was ever put on: the +// buttons are enabled and the form thawed either way, because a face that +// never arrived costs nothing to take off and a state that lies costs a run. +func (b *busy) undress() { + b.worn = false + b.fields.Freeze(false) + for _, control := range b.also { + control.Enable() + } + b.preview.Enable() + b.generate.Enable() + b.cancel.Disable() + b.cancel.Hide() + b.bar.Hide() + b.relay() +} + +// relay lays the row of buttons out again, because the toolkit does not. +// +// Measured in the pinned toolkit on 2026-09-21, from the owner's report that +// Preview and Generate stood off centre after a preview: hiding a child +// changes what the row asks for, so the canvas lays out the row's PARENT +// (internal/driver/common/canvas.go, EnsureMinSize, parentNeedingUpdate) - +// and the parent hands the row the same size it had, which a container +// answers by doing nothing (Container.Resize returns on an unchanged size). +// So the row kept the positions it had worked out with Cancel in it, and the +// two buttons stayed half a Cancel to the left until something else moved +// them. Showing a child goes through the same path and happens to work, +// because the child's own size changes and the row is the parent then. One +// call here for both directions, so the row never depends on which way the +// toolkit happened to walk. +func (b *busy) relay() { + if b.row != nil { + b.row.Refresh() + } +} diff --git a/internal/gui/window/runrefuse.go b/internal/gui/window/runrefuse.go index 6edac3e..c78f21b 100644 --- a/internal/gui/window/runrefuse.go +++ b/internal/gui/window/runrefuse.go @@ -171,7 +171,7 @@ func (r *runner) recheck(setting string) { } // A run owns the screen while it lasts. Its progress and its refusals are // not to be wiped by a keystroke. - if r.running { + if r.busy.occupied { return } // Whatever changed, the line says what the form comes to now - over an