From e70a16c345e4352fa0fb2021ae85dec639d07d8a Mon Sep 17 00:00:00 2001 From: Elias Naur Date: Fri, 22 Jan 2021 16:04:10 +0100 Subject: [PATCH] io/router/key: add explicit tag to FocusOp; make last SoftKeyboardOp apply The target of FocusOp is too subtle; be explicit instead and remove any doubt. Multiple SoftKeyboardOp in a single frame is rare, but if they do occur, they should behave as if they were from separate frames: the last one applies. As a side-effect the key event router can be much simplified. Signed-off-by: Elias Naur --- internal/opconst/ops.go | 4 +- io/key/key.go | 15 ++++--- io/router/key.go | 88 ++++++++++------------------------------- io/router/key_test.go | 38 ++++++------------ widget/editor.go | 2 +- 5 files changed, 41 insertions(+), 106 deletions(-) diff --git a/internal/opconst/ops.go b/internal/opconst/ops.go index 58904ee1..2f146a85 100644 --- a/internal/opconst/ops.go +++ b/internal/opconst/ops.go @@ -54,7 +54,7 @@ const ( TypeClipboardReadLen = 1 TypeClipboardWriteLen = 1 TypeKeyInputLen = 1 - TypeKeyFocusLen = 1 + 1 + TypeKeyFocusLen = 1 TypeKeySoftKeyboardLen = 1 + 1 TypeSaveLen = 1 + 4 TypeLoadLen = 1 + 1 + 4 @@ -115,7 +115,7 @@ func (t OpType) Size() int { func (t OpType) NumRefs() int { switch t { - case TypeKeyInput, TypePointerInput, TypeProfile, TypeCall, TypeClipboardRead, TypeClipboardWrite, TypeCursor: + case TypeKeyInput, TypeKeyFocus, TypePointerInput, TypeProfile, TypeCall, TypeClipboardRead, TypeClipboardWrite, TypeCursor: return 1 case TypeImage: return 2 diff --git a/io/key/key.go b/io/key/key.go index f3cda60d..8b11caf7 100644 --- a/io/key/key.go +++ b/io/key/key.go @@ -26,15 +26,17 @@ type InputOp struct { } // SoftKeyboardOp shows or hide the on-screen keyboard, if available. +// It replaces any previous SoftKeyboardOp. type SoftKeyboardOp struct { Show bool } -// FocusOp sets or clears the keyboard focus. +// FocusOp sets or clears the keyboard focus. It replaces any previous +// FocusOp in the same frame. type FocusOp struct { - // Focus, if set, moves the focus to the current InputOp. If Focus - // is false, the focus is cleared. - Focus bool + // Tag is the new focus. The focus is cleared if Tag is nil, or if Tag + // has no InputOp in the same frame. + Tag event.Tag } // A FocusEvent is generated when a handler gains or loses @@ -132,11 +134,8 @@ func (h SoftKeyboardOp) Add(o *op.Ops) { } func (h FocusOp) Add(o *op.Ops) { - data := o.Write(opconst.TypeKeyFocusLen) + data := o.Write1(opconst.TypeKeyFocusLen, h.Tag) data[0] = byte(opconst.TypeKeyFocus) - if h.Focus { - data[1] = 1 - } } func (EditEvent) ImplementsEvent() {} diff --git a/io/router/key.go b/io/router/key.go index 5f64824c..8d75bb02 100644 --- a/io/router/key.go +++ b/io/router/key.go @@ -17,14 +17,6 @@ type keyQueue struct { handlers map[event.Tag]*keyHandler reader ops.Reader state TextInputState - // states store states during resolveFocus - states []resolveState -} - -type resolveState struct { - tag event.Tag - pri listenerPriority - keyboard TextInputState } type keyHandler struct { @@ -34,15 +26,6 @@ type keyHandler struct { new bool } -type listenerPriority uint8 - -const ( - priDefault listenerPriority = iota - priCurrentFocus - priNone - priNewFocus -) - const ( TextInputKeep TextInputState = iota TextInputClose @@ -64,36 +47,37 @@ func (q *keyQueue) Frame(root *op.Ops, events *handlerEvents) { } q.reader.Reset(root) - state := q.resolveFocus(events) - if state.pri == priNone { - state.tag = nil - } + focus, changed, state := q.resolveFocus(events) for k, h := range q.handlers { if !h.visible { delete(q.handlers, k) if q.focus == k { // Remove the focus from the handler that is no longer visible. q.focus = nil - state.keyboard = TextInputClose + state = TextInputClose } - } - if h.new && k != state.tag { - // Reset the handler on (each) first appearance. + } else if h.new && k != focus { + // Reset the handler on (each) first appearance, but don't trigger redraw. events.Add(k, key.FocusEvent{Focus: false}) } } - if state.tag != q.focus { + if changed && focus != nil { + if _, exists := q.handlers[focus]; !exists { + focus = nil + } + } + if changed && focus != q.focus { if q.focus != nil { events.Add(q.focus, key.FocusEvent{Focus: false}) } - q.focus = state.tag + q.focus = focus if q.focus != nil { events.Add(q.focus, key.FocusEvent{Focus: true}) } else { - state.keyboard = TextInputClose + state = TextInputClose } } - q.state = state.keyboard + q.state = state } func (q *keyQueue) Push(e event.Event, events *handlerEvents) { @@ -102,63 +86,31 @@ func (q *keyQueue) Push(e event.Event, events *handlerEvents) { } } -func (q *keyQueue) resolveFocus(events *handlerEvents) resolveState { - var state resolveState +func (q *keyQueue) resolveFocus(events *handlerEvents) (focus event.Tag, changed bool, state TextInputState) { for encOp, ok := q.reader.Decode(); ok; encOp, ok = q.reader.Decode() { switch opconst.OpType(encOp.Data[0]) { case opconst.TypeKeyFocus: op := decodeFocusOp(encOp.Data, encOp.Refs) - if op.Focus { - state.pri = priNewFocus - } else { - state.pri, state.keyboard = priNone, TextInputClose - } + changed = true + focus = op.Tag case opconst.TypeKeySoftKeyboard: op := decodeSoftKeyboardOp(encOp.Data, encOp.Refs) if op.Show { - state.keyboard = TextInputOpen + state = TextInputOpen } else { - state.keyboard = TextInputClose + state = TextInputClose } case opconst.TypeKeyInput: op := decodeKeyInputOp(encOp.Data, encOp.Refs) - if op.Tag == q.focus && state.pri < priCurrentFocus { - state.pri = priCurrentFocus - } h, ok := q.handlers[op.Tag] if !ok { h = &keyHandler{new: true} q.handlers[op.Tag] = h } h.visible = true - state.tag = op.Tag - case opconst.TypeSave: - id := ops.DecodeSave(encOp.Data) - if extra := id - len(q.states) + 1; extra > 0 { - q.states = append(q.states, make([]resolveState, extra)...) - } - q.states[id] = state - state = resolveState{} - case opconst.TypeLoad: - id, mask := ops.DecodeLoad(encOp.Data) - restored := q.states[id] - if state.keyboard > restored.keyboard { - restored.keyboard = state.keyboard - } - if state.pri.replaces(restored.pri) { - restored.tag, restored.pri = state.tag, state.pri - } - if mask != 0 { - state = restored - } } } - return state -} - -func (p listenerPriority) replaces(p2 listenerPriority) bool { - // Favor earliest default focus or latest requested focus. - return p > p2 || p == p2 && p == priNewFocus + return } func decodeKeyInputOp(d []byte, refs []interface{}) key.InputOp { @@ -184,6 +136,6 @@ func decodeFocusOp(d []byte, refs []interface{}) key.FocusOp { panic("invalid op") } return key.FocusOp{ - Focus: d[1] != 0, + Tag: refs[0], } } diff --git a/io/router/key_test.go b/io/router/key_test.go index 0e975edb..bcdc677b 100644 --- a/io/router/key_test.go +++ b/io/router/key_test.go @@ -18,7 +18,7 @@ func TestKeyMultiples(t *testing.T) { key.SoftKeyboardOp{Show: true}.Add(ops) key.InputOp{Tag: &handlers[0]}.Add(ops) - key.FocusOp{Focus: true}.Add(ops) + key.FocusOp{Tag: &handlers[2]}.Add(ops) key.InputOp{Tag: &handlers[1]}.Add(ops) // The last one must be focused: @@ -40,27 +40,19 @@ func TestKeyStacked(t *testing.T) { s := op.Save(ops) key.InputOp{Tag: &handlers[0]}.Add(ops) - // FocusOp must not overwrite the - // FocusOp{Focus: true}. - key.FocusOp{Focus: false}.Add(ops) + key.FocusOp{Tag: nil}.Add(ops) s.Load() s = op.Save(ops) key.SoftKeyboardOp{Show: false}.Add(ops) key.InputOp{Tag: &handlers[1]}.Add(ops) - key.FocusOp{Focus: true}.Add(ops) + key.FocusOp{Tag: &handlers[1]}.Add(ops) s.Load() s = op.Save(ops) key.InputOp{Tag: &handlers[2]}.Add(ops) - // SoftwareKeyboardOp will open the keyboard, - // overwriting `SoftKeyboardOp{Show: false}`. key.SoftKeyboardOp{Show: true}.Add(ops) s.Load() s = op.Save(ops) - key.SoftKeyboardOp{Show: false}.Add(ops) key.InputOp{Tag: &handlers[3]}.Add(ops) - // FocusOp must not overwrite the - // FocusOp{Focus: true}. - key.FocusOp{Focus: false}.Add(ops) s.Load() r.Frame(ops) @@ -95,7 +87,7 @@ func TestKeyRemoveFocus(t *testing.T) { // New InputOp with Focus and Keyboard: s := op.Save(ops) key.InputOp{Tag: &handlers[0]}.Add(ops) - key.FocusOp{Focus: true}.Add(ops) + key.FocusOp{Tag: &handlers[0]}.Add(ops) key.SoftKeyboardOp{Show: true}.Add(ops) s.Load() @@ -127,14 +119,13 @@ func TestKeyRemoveFocus(t *testing.T) { key.InputOp{Tag: &handlers[1]}.Add(ops) s.Load() - // Removing any Focus: + // Remove focus by focusing on a tag that don't exist. s = op.Save(ops) - key.FocusOp{Focus: false}.Add(ops) + key.FocusOp{Tag: new(int)}.Add(ops) s.Load() r.Frame(ops) - assertKeyEvent(t, r.Events(&handlers[0]), false) assertKeyEventUnexpected(t, r.Events(&handlers[1])) assertFocus(t, r, nil) assertKeyboard(t, r, TextInputClose) @@ -145,11 +136,6 @@ func TestKeyRemoveFocus(t *testing.T) { key.InputOp{Tag: &handlers[0]}.Add(ops) s.Load() - // Setting Focus without InputOp: - s = op.Save(ops) - key.FocusOp{Focus: true}.Add(ops) - s.Load() - s = op.Save(ops) key.InputOp{Tag: &handlers[1]}.Add(ops) s.Load() @@ -166,23 +152,21 @@ func TestKeyRemoveFocus(t *testing.T) { // Set focus to InputOp which already // exists in the previous frame: s = op.Save(ops) - key.FocusOp{Focus: true}.Add(ops) + key.FocusOp{Tag: &handlers[0]}.Add(ops) key.InputOp{Tag: &handlers[0]}.Add(ops) key.SoftKeyboardOp{Show: true}.Add(ops) s.Load() - // Tries to remove focus: - // It must not overwrite the previous `FocusOp`. + // Remove focus. s = op.Save(ops) key.InputOp{Tag: &handlers[1]}.Add(ops) - key.FocusOp{Focus: false}.Add(ops) + key.FocusOp{Tag: nil}.Add(ops) s.Load() r.Frame(ops) - assertKeyEvent(t, r.Events(&handlers[0]), true) assertKeyEventUnexpected(t, r.Events(&handlers[1])) - assertFocus(t, r, &handlers[0]) + assertFocus(t, r, nil) assertKeyboard(t, r, TextInputOpen) } @@ -193,7 +177,7 @@ func TestKeyFocusedInvisible(t *testing.T) { // Set new InputOp with focus: s := op.Save(ops) - key.FocusOp{Focus: true}.Add(ops) + key.FocusOp{Tag: &handlers[0]}.Add(ops) key.InputOp{Tag: &handlers[0]}.Add(ops) key.SoftKeyboardOp{Show: true}.Add(ops) s.Load() diff --git a/widget/editor.go b/widget/editor.go index a1673b42..7e04509e 100644 --- a/widget/editor.go +++ b/widget/editor.go @@ -416,7 +416,7 @@ func (e *Editor) layout(gtx layout.Context) layout.Dimensions { key.InputOp{Tag: &e.eventKey}.Add(gtx.Ops) if e.requestFocus { - key.FocusOp{Focus: true}.Add(gtx.Ops) + key.FocusOp{Tag: &e.eventKey}.Add(gtx.Ops) key.SoftKeyboardOp{Show: true}.Add(gtx.Ops) } e.requestFocus = false