Skip to content

Commit e14bd15

Browse files
donislawdevclaude
andcommitted
gui: a press puts the keyboard on a switch, a frozen control answers no key
Four findings of an outside review of the pull request, each checked against the toolkit's source before anything was changed, and one more found beside them. A press on a switch - the square and the segmented one - now puts the keyboard on it quietly, and the first key drawn on it turns the ring on: the rule PointerFocus has stated since the menus, which the Chooser followed and the two controls of the rework did not. The desktop driver unfocuses whatever had the keyboard on a press and leaves focusing to the widget (internal/driver/glfw/window.go, mouseClicked); the toolkit's own Check and radio item focus themselves in Tapped, and ours did not, so after a click the keyboard was nowhere - the next Space flipped nothing and the next Tab started from the top of the screen. The review said the keyboard stayed on the previous control, which the driver does not do, and drew the right conclusion from the wrong mechanism. The guard that promised "a press moves the keyboard without drawing its mark" asked only about the mark, so a control that took the keyboard nowhere passed it. It asks both now. A frozen control answers no key. The focus manager asks Disabled only when it moves the focus, and the driver hands every key to whatever is focused, so a segmented switch that had the keyboard when a run began went on moving its choice under a form drawn as frozen. The review named the switch. The menu does the same through the toolkit's Select.TypedKey, which asks nothing either, and is stopped in the same place. One press of the space bar flips a switch once. Toggle.TypedRune answered the space character as Button.TypedRune did, so a press flipped the switch twice and left it where it was, which reads as a switch that ignores space. Found by reading the control beside the one the review named. An ownWork exemption that no build spends is red, as a registry entry that no build matches already was: a key for a file that has gone would exempt whatever landed at that path next. The catalogue no longer writes its window size down as the size the ordinary window opens at. The comment beside it said nothing about it was remembered, and the close callback two lines below remembered it. Behind cgo, like the rest of the remembering, so no guard reaches it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 8c8b550 commit e14bd15

7 files changed

Lines changed: 256 additions & 19 deletions

File tree

‎internal/guard/controlstates_test.go‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,58 @@ func TestOnePressOfSpacePressesAButtonOnce(t *testing.T) {
111111
}
112112
}
113113

114+
// And flips a switch once - the same two deliveries, and a switch that
115+
// answered both flipped twice, back to where it started, which reads as a
116+
// switch that ignores the space bar. Found by reading the switch beside the
117+
// button on 2026-09-16, not by the review that named the button.
118+
func TestOnePressOfSpaceFlipsASwitchOnce(t *testing.T) {
119+
flips := 0
120+
s := parts.NewToggle(func(bool) { flips++ })
121+
s.TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace})
122+
s.TypedRune(' ')
123+
if flips != 1 || !s.Checked {
124+
t.Errorf("one press of the space bar flipped the switch %d times and left it %v", flips, s.Checked)
125+
}
126+
}
127+
128+
// A frozen control answers no key.
129+
//
130+
// A form is frozen for the length of a run (Fields.Freeze disables every
131+
// control), and a control that had the keyboard keeps it: the focus manager
132+
// asks Disabled only when it MOVES the focus (internal/app/focus_manager.go)
133+
// and the driver hands every key to whatever is focused. So a disabled
134+
// switch went on moving its choice under a form drawn as frozen, and a
135+
// disabled menu went on changing its value on Left and Right through the
136+
// toolkit's own Select.TypedKey, which asks nothing either. An outside review
137+
// of the pull request named the switch on 2026-09-16. The menu came out of
138+
// reading what the switch's fix had to cover.
139+
func TestAFrozenControlAnswersNoKey(t *testing.T) {
140+
changed := 0
141+
segments := parts.NewSegments([]string{"one", "two", "three"}, func(string) { changed++ })
142+
segments.Disable()
143+
segments.TypedKey(&fyne.KeyEvent{Name: fyne.KeyRight})
144+
if segments.Selected != "one" || changed != 0 {
145+
t.Errorf("a frozen segmented switch moved to %q on an arrow and reported %d change(s)", segments.Selected, changed)
146+
}
147+
148+
menu := parts.NewChooser([]string{"one", "two", "three"}, func(string) { changed++ })
149+
menu.SetSelected("one")
150+
changed = 0
151+
menu.Disable()
152+
menu.TypedKey(&fyne.KeyEvent{Name: fyne.KeyRight})
153+
if menu.Selected != "one" || changed != 0 {
154+
t.Errorf("a frozen menu moved to %q on an arrow and reported %d change(s)", menu.Selected, changed)
155+
}
156+
157+
toggle := parts.NewToggle(func(bool) { changed++ })
158+
changed = 0
159+
toggle.Disable()
160+
toggle.TypedKey(&fyne.KeyEvent{Name: fyne.KeySpace})
161+
if toggle.Checked || changed != 0 {
162+
t.Errorf("a frozen switch flipped on the space bar and reported %d change(s)", changed)
163+
}
164+
}
165+
114166
// A segmented switch ignores a value it does not hold.
115167
//
116168
// A switch of fixed choices is not a box: handed a word it does not offer it

‎internal/guard/embeddedassets_test.go‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,18 @@ func TestEveryFileEmbeddedFromSomebodyElseIsAccountedFor(t *testing.T) {
3131
seen := map[string]bool{}
3232
matched := map[string]bool{}
3333
total := 0
34+
// Every key of ownWork has to be spent on a file some build embeds, or it
35+
// is an exemption that outlived its file - and an exemption nobody uses
36+
// today is the one that exempts somebody else's bytes tomorrow, at the
37+
// same package and path, without a word. Asked the same way the registry
38+
// is asked below, since an unused entry and an unused exemption are the
39+
// same drift facing two ways. An outside review of the pull request
40+
// pointed at the missing half on 2026-09-16.
41+
usedExemptions := map[string]bool{}
3442

3543
for _, target := range []string{"../../cmd/tfg-gui", "../../cmd/tfg"} {
3644
for _, goos := range []string{"windows", "linux", "darwin"} {
37-
total += accountForBuild(t, target, goos, seen, matched)
45+
total += accountForBuild(t, target, goos, seen, matched, usedExemptions)
3846
}
3947
}
4048

@@ -59,6 +67,18 @@ func TestEveryFileEmbeddedFromSomebodyElseIsAccountedFor(t *testing.T) {
5967
"An entry for bytes that no longer ship is a notice nobody needs, and it hides the day "+
6068
"the real thing was replaced by something else.", len(stale), strings.Join(stale, "\n "))
6169
}
70+
var unspent []string
71+
for key := range ownWork {
72+
if !usedExemptions[key] {
73+
unspent = append(unspent, key)
74+
}
75+
}
76+
if len(unspent) > 0 {
77+
sort.Strings(unspent)
78+
t.Errorf("%d ownWork exemption(s) name a file no build embeds:\n %s\n"+
79+
"An exemption without a file is a licence check switched off for whatever lands at that "+
80+
"path next. Take it off the list.", len(unspent), strings.Join(unspent, "\n "))
81+
}
6282
t.Logf("%d embedded file(s) across every package, all accounted for by %d registry entr(y/ies) or named as our own work",
6383
total, len(legal.Assets()))
6484
}
@@ -67,19 +87,19 @@ func TestEveryFileEmbeddedFromSomebodyElseIsAccountedFor(t *testing.T) {
6787
// added. Split out rather than nested inside the test because the depth ceiling
6888
// said so, and the ceiling is a measurement rather than a preference - see
6989
// docs/QUALITY.md.
70-
func accountForBuild(t *testing.T, target, goos string, seen, matched map[string]bool) int {
90+
func accountForBuild(t *testing.T, target, goos string, seen, matched, usedExemptions map[string]bool) int {
7191
t.Helper()
7292
added := 0
7393
for pkg, files := range embeddedFiles(t, target, goos) {
74-
added += accountForPackage(t, pkg, files, seen, matched)
94+
added += accountForPackage(t, pkg, files, seen, matched, usedExemptions)
7595
}
7696
return added
7797
}
7898

7999
// accountForPackage does one package, skipping what another platform already
80100
// answered for. Three systems are asked and they agree about most of the tree,
81101
// so without seen the same font would be reported six times.
82-
func accountForPackage(t *testing.T, pkg string, files []string, seen, matched map[string]bool) int {
102+
func accountForPackage(t *testing.T, pkg string, files []string, seen, matched, usedExemptions map[string]bool) int {
83103
t.Helper()
84104
added := 0
85105
for _, file := range files {
@@ -89,6 +109,7 @@ func accountForPackage(t *testing.T, pkg string, files []string, seen, matched m
89109
seen[pkg+" "+file] = true
90110
added++
91111
if ownWork[pkg+" "+file] {
112+
usedExemptions[pkg+" "+file] = true
92113
continue
93114
}
94115
accountFor(t, pkg, file, matched)

‎internal/guard/pointerfocus_test.go‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,16 @@ func TestAPressMovesTheKeyboardWithoutDrawingItsMark(t *testing.T) {
4343
t.Error("the format menu was pressed with the pointer and drew the keyboard mark, " +
4444
"so the value chosen sits on a blue box until something else is clicked")
4545
}
46+
// And the keyboard IS there. The name of this guard promised that from
47+
// the start and until 2026-09-16 only the second half was asked, so a
48+
// control that took the keyboard nowhere at all passed it - which is what
49+
// the switch below did: the driver unfocuses whatever had the keyboard on
50+
// a press and leaves focusing to the widget, and the switch left it.
51+
// On the list the press opened, which is where the arrows have to work.
52+
// Closing it hands the keyboard back to the menu (see Chooser.giveBack).
53+
if list := menu.Opened(); list == nil || c.Focused() != fyne.Focusable(list) {
54+
t.Errorf("the format menu was pressed and the keyboard is on %T, not on the list it opened", c.Focused())
55+
}
4656

4757
// The list the press opened is taken away first. A real press respects what
4858
// covers it - which is the point of pressing for real - so leaving an open
@@ -71,6 +81,46 @@ func TestAPressMovesTheKeyboardWithoutDrawingItsMark(t *testing.T) {
7181
t.Error("the switch was pressed with the pointer and drew the keyboard mark, " +
7282
"which is the blue disc behind the square that was reported on 2026-08-18")
7383
}
84+
if c.Focused() != toggle {
85+
t.Errorf("the switch was pressed and the keyboard is on %T, not on the switch - so the next "+
86+
"Space flips nothing and the next Tab starts from the top of the screen", c.Focused())
87+
}
88+
}
89+
90+
// A press on a segmented switch puts the keyboard on it quietly, and the
91+
// first arrow says so.
92+
//
93+
// The same rule as the two above, on the third control that can hold the
94+
// keyboard. Asked on a bare canvas rather than on a screen, because a press
95+
// aimed at one segment needs a position, and the segment widths are the
96+
// control's own to know.
97+
func TestAPressOnASegmentedSwitchPutsTheKeyboardOnItQuietly(t *testing.T) {
98+
app := test.NewApp()
99+
app.Settings().SetTheme(parts.Theme())
100+
t.Cleanup(func() { test.NewApp() })
101+
102+
s := parts.NewSegments([]string{"one", "two", "three"}, nil)
103+
w := test.NewWindow(s)
104+
t.Cleanup(w.Close)
105+
w.Resize(s.MinSize())
106+
107+
test.TapAt(s, fyne.NewPos(s.Size().Width-1, s.Size().Height/2))
108+
if s.Selected != "three" {
109+
t.Fatalf("the press on the last segment chose %q, so this guard never reached the state it is about", s.Selected)
110+
}
111+
if w.Canvas().Focused() != s {
112+
t.Errorf("the switch was pressed and the keyboard is on %T, not on the switch", w.Canvas().Focused())
113+
}
114+
if s.Marked() {
115+
t.Error("the switch was pressed with the pointer and drew the keyboard mark")
116+
}
117+
s.TypedKey(&fyne.KeyEvent{Name: fyne.KeyLeft})
118+
if !s.Marked() {
119+
t.Error("an arrow was pressed on the switch and the mark that says the keyboard is here did not come on")
120+
}
121+
if s.Selected != "two" {
122+
t.Errorf("the arrow after the press moved to %q, not on from the segment that was pressed", s.Selected)
123+
}
74124
}
75125

76126
// The other half, and it is the half that makes the first one safe.

‎internal/gui/parts/ring.go‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,13 @@ func (c *Chooser) Quietly(focus func()) { c.from.Quietly(focus) }
529529
// UX9 asks that whatever the mouse can do the keyboard can - which has to mean
530530
// the same thing, not a second version of it.
531531
func (c *Chooser) TypedKey(event *fyne.KeyEvent) {
532+
// A frozen menu answers no key. The toolkit's Select.TypedKey moves the
533+
// value on Left and Right without asking, and a menu frozen for a run
534+
// keeps the keyboard if it had it - see Segments.TypedKey for the same
535+
// finding on the same day.
536+
if event == nil || c.Disabled() {
537+
return
538+
}
532539
// The keyboard has been used, so from here on it is worth saying where it
533540
// is. Somebody who opened this list with the mouse and then reached for the
534541
// arrows is somebody who now needs to see which control is listening.

‎internal/gui/parts/segments.go‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ type Segments struct {
3838

3939
hovered int // the segment under the pointer, or -1
4040
marked bool
41+
// from knows whether the keyboard arrived by press or by key - see
42+
// PointerFocus and Toggle, which follow the same rule.
43+
from PointerFocus
4144
}
4245

4346
var (
@@ -85,16 +88,38 @@ func (s *Segments) indexOf(value string) int {
8588
return -1
8689
}
8790

88-
// Tapped chooses the segment under the pointer.
91+
// Tapped chooses the segment under the pointer and puts the keyboard on the
92+
// switch, quietly - the way the toolkit's radio item does (widget/radio_item.go
93+
// Tapped), so the arrows step on from what was clicked. See Toggle.Tapped for
94+
// the whole of why, and for what stood here until 2026-09-16.
8995
func (s *Segments) Tapped(event *fyne.PointEvent) {
9096
if s.Disabled() || event == nil {
9197
return
9298
}
99+
s.takeTheKeyboardQuietly()
93100
if at := s.segmentAt(event.Position.X); at >= 0 {
94101
s.SetSelected(s.Options[at])
95102
}
96103
}
97104

105+
// takeTheKeyboardQuietly moves the focus here without the mark, unless it is
106+
// here already.
107+
func (s *Segments) takeTheKeyboardQuietly() {
108+
app := fyne.CurrentApp()
109+
if app == nil {
110+
return
111+
}
112+
canvas := app.Driver().CanvasForObject(s)
113+
if canvas == nil || canvas.Focused() == s {
114+
return
115+
}
116+
s.from.Quietly(func() { canvas.Focus(s) })
117+
}
118+
119+
// Quietly runs a focus change without drawing the mark. See PointerFocus and
120+
// FocusQuietly.
121+
func (s *Segments) Quietly(focus func()) { s.from.Quietly(focus) }
122+
98123
// segmentAt is the segment a point falls in, or -1 past the last one.
99124
func (s *Segments) segmentAt(x float32) int {
100125
edge := float32(0)
@@ -130,7 +155,16 @@ func (s *Segments) MouseOut() {
130155
}
131156
}
132157

158+
// FocusGained draws the ring when the keyboard is what brought the focus here.
159+
// A press brings it quietly and the first key turns the ring on.
133160
func (s *Segments) FocusGained() {
161+
if s.from.Quiet() {
162+
return
163+
}
164+
s.mark()
165+
}
166+
167+
func (s *Segments) mark() {
134168
s.marked = true
135169
s.Refresh()
136170
}
@@ -147,9 +181,18 @@ func (s *Segments) TypedRune(rune) {}
147181
// tab strip makes (see TabWord) not applying where the choice does not move the
148182
// keyboard off the control.
149183
func (s *Segments) TypedKey(event *fyne.KeyEvent) {
150-
if event == nil || len(s.Options) == 0 {
184+
// Disabled asked here and not left to the renderer: a switch frozen for
185+
// the length of a run keeps the keyboard if it had it, because the focus
186+
// manager checks Disabled only when it MOVES the focus (internal/app/
187+
// focus_manager.go), and the driver hands every key to whatever is
188+
// focused. Without this the arrows moved the choice under a form drawn as
189+
// frozen - an outside review of the pull request named it, 2026-09-16.
190+
if event == nil || s.Disabled() || len(s.Options) == 0 {
151191
return
152192
}
193+
if !s.marked {
194+
s.mark()
195+
}
153196
at := s.indexOf(s.Selected)
154197
switch event.Name {
155198
case fyne.KeyLeft:

0 commit comments

Comments
 (0)