widget: click button only if key pressed and released #120
Conversation
Can you add a test, say in I believe this should be with a slash: |
Done, to the best of my ability, but I have a feeling there's cleaner way to write the test.
Changed to a slash. I got the # syntax from here https://gioui.org/doc/contribute so assuming slash is correct, there's likely a typo in the instructions. |
There was a problem hiding this comment.
LGTM, apart from nits. @whereswaldon do you have any objections to this behaviour change?
widget/button_test.go
Outdated
| "gioui.org/widget" | ||
| ) | ||
|
|
||
| func TestButtonKeypresses(t *testing.T) { |
There was a problem hiding this comment.
Nit: rename to TestClickable.
widget/button_test.go
Outdated
| layout := func() { | ||
| layout.Flex{Axis: layout.Horizontal}.Layout(gtx, | ||
| layout.Rigid(func(gtx layout.Context) layout.Dimensions { | ||
| return b1.Layout(gtx, func(gtx layout.Context) layout.Dimensions { | ||
| return layout.Dimensions{Size: image.Pt(100, 100)} | ||
| }) | ||
| }), | ||
| layout.Rigid(func(gtx layout.Context) layout.Dimensions { | ||
| return b2.Layout(gtx, func(gtx layout.Context) layout.Dimensions { | ||
| return layout.Dimensions{Size: image.Pt(100, 100)} | ||
| }) | ||
| }), | ||
| ) | ||
| } |
There was a problem hiding this comment.
If you only need to offset b2, layout can be written without Flex, by using op.Offset just before laying out b2:
defer op.Offset(image.Pt(100, 100)).Push(gtx.Ops).Pop()
No, I wholeheartedly agree with it. |
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>
|
Done the nits. I didn't even offset the buttons; just laid them out on top of each other as the test doesn't click anything, just uses Focus, Focused and keyboard presses, so it doesn't make a difference. Didn't want to add anything unnecessary to the test. |
|
Merged, thank you!
Fixed. Thanks for pointing that out. |
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