From 44ec48d3254575e9ac6ec01a9ace72262d4f3147 Mon Sep 17 00:00:00 2001 From: Chris Waldon Date: Tue, 31 May 2022 17:04:36 -0400 Subject: [PATCH] io/router: fix focused key event propagation When a key.InputOp is focused, keypresses that it does not explicitly include in its key set should check for ancestor clip areas that are interested in them. Previously this check only included ancestors of the final clip area in the hit tree, and could fail to find ancestors of the focused key.InputOp because they were in a different branch. This commit also adds a test to lock in the new behavior. This can likely be made more efficient by adding a rapid way to map from the focused key tag to its index in the hit tree. I wasn't sure whether the complexity was warranted, but I'm happy to do that if it's desired. Signed-off-by: Chris Waldon --- io/router/key_test.go | 40 ++++++++++++++++++++++++++++++++++------ io/router/router.go | 9 ++++++++- 2 files changed, 42 insertions(+), 7 deletions(-) diff --git a/io/router/key_test.go b/io/router/key_test.go index 4bab0f1c..522f197a 100644 --- a/io/router/key_test.go +++ b/io/router/key_test.go @@ -321,37 +321,65 @@ func TestNoFocus(t *testing.T) { } func TestKeyRouting(t *testing.T) { - handlers := make([]int, 3) + handlers := make([]int, 5) ops := new(op.Ops) + macroOps := new(op.Ops) r := new(Router) rect := clip.Rect{Max: image.Pt(10, 10)} + macro := op.Record(macroOps) key.InputOp{Tag: &handlers[0], Keys: "A"}.Add(ops) cl1 := rect.Push(ops) key.InputOp{Tag: &handlers[1], Keys: "B"}.Add(ops) key.InputOp{Tag: &handlers[2], Keys: "A"}.Add(ops) cl1.Pop() + cl2 := rect.Push(ops) + key.InputOp{Tag: &handlers[3], Keys: "B"}.Add(ops) + key.InputOp{Tag: &handlers[4], Keys: "A"}.Add(ops) + cl2.Pop() + call := macro.Stop() + call.Add(ops) r.Frame(ops) A, B := key.Event{Name: "A"}, key.Event{Name: "B"} r.Queue(A, B) - assertKeyEvent(t, r.Events(&handlers[2]), false, A) - assertKeyEvent(t, r.Events(&handlers[1]), false, B) + // With no focus, the events should traverse the final branch of the hit tree + // searching for handlers. + assertKeyEvent(t, r.Events(&handlers[4]), false, A) + assertKeyEvent(t, r.Events(&handlers[3]), false, B) + assertKeyEvent(t, r.Events(&handlers[2]), false) + assertKeyEvent(t, r.Events(&handlers[1]), false) assertKeyEvent(t, r.Events(&handlers[0]), false) + + r2 := new(Router) + + call.Add(ops) + key.FocusOp{Tag: &handlers[2]}.Add(ops) + r2.Frame(ops) + + r2.Queue(A, B) + + // With focus, the events should traverse the branch of the hit tree + // containing the focused element. + assertKeyEvent(t, r2.Events(&handlers[4]), false) + assertKeyEvent(t, r2.Events(&handlers[3]), false) + assertKeyEvent(t, r2.Events(&handlers[2]), true, A) + assertKeyEvent(t, r2.Events(&handlers[1]), false, B) + assertKeyEvent(t, r2.Events(&handlers[0]), false) } -func assertKeyEvent(t *testing.T, events []event.Event, expected bool, expectedInputs ...event.Event) { +func assertKeyEvent(t *testing.T, events []event.Event, expectedFocus bool, expectedInputs ...event.Event) { t.Helper() var evtFocus int var evtKeyPress int for _, e := range events { switch ev := e.(type) { case key.FocusEvent: - if ev.Focus != expected { - t.Errorf("focus is expected to be %v, got %v", expected, ev.Focus) + if ev.Focus != expectedFocus { + t.Errorf("focus is expected to be %v, got %v", expectedFocus, ev.Focus) } evtFocus++ case key.Event, key.EditEvent: diff --git a/io/router/router.go b/io/router/router.go index 018b6a3e..4271cbb4 100644 --- a/io/router/router.go +++ b/io/router/router.go @@ -183,12 +183,19 @@ func rangeNorm(r key.Range) key.Range { func (q *Router) queueKeyEvent(e key.Event) { kq := &q.key.queue - if f := q.key.queue.focus; f != nil && kq.Accepts(f, e) { + f := q.key.queue.focus + if f != nil && kq.Accepts(f, e) { q.handlers.Add(f, e) return } pq := &q.pointer.queue idx := len(pq.hitTree) - 1 + if f != nil { + // If there is a focused tag, traverse its ancestry through the + // hit tree to search for handlers. + for ; pq.hitTree[idx].ktag != f; idx-- { + } + } for idx != -1 { n := &pq.hitTree[idx] idx = n.next