widget: click button only if key pressed and released

This commit fixes the non-intuitive behaviour, where hitting return or
space with a button focused, then tabbing to another button and
releasing the key causes the second button to trigger. It feels wrong,
as the "gesture" was never initiated on the second button. The fix makes
widget.Clickable track which key was pressed, in a variable called
pressedKey, and only considers a key release if the released key matches
the pressed key. Finally, if the widget loses focus, pressedKey is
cleared.

Fixes: https://todo.sr.ht/~eliasnaur/gio/525
Signed-off-by: Veikko Sariola <5684185+vsariola@users.noreply.github.com>

widget/button.go

@@ -28,6 +28,7 @@ type Clickable struct {
 	keyTag       struct{}
 	requestFocus bool
 	focused      bool
+	pressedKey   string
 }
 
 // Click represents a click.
@@ -178,17 +179,30 @@ func (b *Clickable) update(gtx layout.Context) {
 		switch e := e.(type) {
 		case key.FocusEvent:
 			b.focused = e.Focus
+			if !b.focused {
+				b.pressedKey = ""
+			}
 		case key.Event:
-			if !b.focused || e.State != key.Release {
+			if !b.focused {
 				break
 			}
 			if e.Name != key.NameReturn && e.Name != key.NameSpace {
 				break
 			}
-			b.clicks = append(b.clicks, Click{
-				Modifiers: e.Modifiers,
-				NumClicks: 1,
-			})
+			switch e.State {
+			case key.Press:
+				b.pressedKey = e.Name
+			case key.Release:
+				if b.pressedKey != e.Name {
+					break
+				}
+				// only register a key as a click if the key was pressed and released while this button was focused
+				b.pressedKey = ""
+				b.clicks = append(b.clicks, Click{
+					Modifiers: e.Modifiers,
+					NumClicks: 1,
+				})
+			}
 		}
 	}
 }

widget/button_test.go

@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: Unlicense OR MIT
+
+package widget_test
+
+import (
+	"image"
+	"testing"
+
+	"gioui.org/io/key"
+	"gioui.org/io/router"
+	"gioui.org/io/system"
+	"gioui.org/layout"
+	"gioui.org/op"
+	"gioui.org/widget"
+)
+
+func TestClickable(t *testing.T) {
+	var (
+		ops op.Ops
+		r   router.Router
+		b1  widget.Clickable
+		b2  widget.Clickable
+	)
+	gtx := layout.NewContext(&ops, system.FrameEvent{Queue: &r})
+	layout := func() {
+		b1.Layout(gtx, func(gtx layout.Context) layout.Dimensions {
+			return layout.Dimensions{Size: image.Pt(100, 100)}
+		})
+		// buttons are on top of each other but we only use focus and keyevents, so this is fine
+		b2.Layout(gtx, func(gtx layout.Context) layout.Dimensions {
+			return layout.Dimensions{Size: image.Pt(100, 100)}
+		})
+	}
+	frame := func() {
+		ops.Reset()
+		layout()
+		r.Frame(gtx.Ops)
+	}
+	// frame: request focus for button 1
+	b1.Focus()
+	frame()
+	// frame: gain focus for button 1
+	frame()
+	if !b1.Focused() {
+		t.Error("button 1 did not gain focus")
+	}
+	if b2.Focused() {
+		t.Error("button 2 should not have focus")
+	}
+	// frame: press & release return
+	r.Queue(
+		key.Event{
+			Name:  key.NameReturn,
+			State: key.Press,
+		},
+		key.Event{
+			Name:  key.NameReturn,
+			State: key.Release,
+		},
+	)
+	frame()
+	if !b1.Clicked() {
+		t.Error("button 1 did not get clicked when it got return press & release")
+	}
+	if b2.Clicked() {
+		t.Error("button 2 got clicked when it did not have focus")
+	}
+	// frame: press return down
+	r.Queue(
+		key.Event{
+			Name:  key.NameReturn,
+			State: key.Press,
+		},
+	)
+	frame()
+	if b1.Clicked() {
+		t.Error("button 1 got clicked, even if it only got return press")
+	}
+	// frame: request focus for button 2
+	b2.Focus()
+	frame()
+	// frame: gain focus for button 2
+	frame()
+	if b1.Focused() {
+		t.Error("button 1 should not have focus")
+	}
+	if !b2.Focused() {
+		t.Error("button 2 did not gain focus")
+	}
+	// frame: release return
+	r.Queue(
+		key.Event{
+			Name:  key.NameReturn,
+			State: key.Release,
+		},
+	)
+	frame()
+	if b1.Clicked() {
+		t.Error("button 1 got clicked, even if it had lost focus")
+	}
+	if b2.Clicked() {
+		t.Error("button 2 should not have been clicked, as it only got return release")
+	}
+}