From f07f4e85340fd1dca397f80adc7bbb0ec7bc81f4 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Sat, 28 Nov 2020 14:57:53 -0500 Subject: [PATCH 1/2] Improving focus tests. The test cases can now use trees with any number of containers and have a scalable way of referring to individual containers in the tree. --- container/focus_test.go | 152 +++++++++++++++++++++++----------------- 1 file changed, 86 insertions(+), 66 deletions(-) diff --git a/container/focus_test.go b/container/focus_test.go index 5f3a5a4..cb5ab1c 100644 --- a/container/focus_test.go +++ b/container/focus_test.go @@ -17,6 +17,7 @@ package container import ( "fmt" "image" + "strings" "testing" "time" @@ -260,6 +261,21 @@ func TestPointCont(t *testing.T) { } } +// contLocIntro prints out an introduction explaining the used container +// locations on test failures. +func contLocIntro() string { + var s strings.Builder + s.WriteString("Container locations refer to containers in the following three, i.e. contLocA is the root container:\n") + s.WriteString(` + A + / \ + B C + / \ +D E +`) + return s.String() +} + // contLoc is used in tests to indicate the desired location of a container. type contLoc int @@ -273,27 +289,29 @@ func (cl contLoc) String() string { // contLocNames maps contLoc values to human readable names. var contLocNames = map[contLoc]string{ - contLocRoot: "Root", - contLocLeft: "Left", - contLocRight: "Right", + contLocA: "contLocA", + contLocB: "contLocB", + contLocC: "contLocC", } const ( contLocUnknown contLoc = iota - contLocRoot - contLocLeft - contLocRight + contLocA + contLocB + contLocC ) func TestFocusTrackerMouse(t *testing.T) { + t.Log(contLocIntro()) + ft, err := faketerm.New(image.Point{10, 10}) if err != nil { t.Fatalf("faketerm.New => unexpected error: %v", err) } var ( - insideLeft = image.Point{1, 1} - insideRight = image.Point{6, 6} + insideB = image.Point{1, 1} + insideC = image.Point{6, 6} ) tests := []struct { @@ -305,7 +323,7 @@ func TestFocusTrackerMouse(t *testing.T) { }{ { desc: "initially the root is focused", - wantFocused: contLocRoot, + wantFocused: contLocA, }, { desc: "click and release moves focus to the left", @@ -313,7 +331,7 @@ func TestFocusTrackerMouse(t *testing.T) { {Position: image.Point{0, 0}, Button: mouse.ButtonLeft}, {Position: image.Point{1, 1}, Button: mouse.ButtonRelease}, }, - wantFocused: contLocLeft, + wantFocused: contLocB, wantProcessed: 2, }, { @@ -322,50 +340,50 @@ func TestFocusTrackerMouse(t *testing.T) { {Position: image.Point{5, 5}, Button: mouse.ButtonLeft}, {Position: image.Point{6, 6}, Button: mouse.ButtonRelease}, }, - wantFocused: contLocRight, + wantFocused: contLocC, wantProcessed: 2, }, { desc: "click in the same container is a no-op", events: []*terminalapi.Mouse{ - {Position: insideRight, Button: mouse.ButtonLeft}, - {Position: insideRight, Button: mouse.ButtonRelease}, - {Position: insideRight, Button: mouse.ButtonLeft}, - {Position: insideRight, Button: mouse.ButtonRelease}, + {Position: insideC, Button: mouse.ButtonLeft}, + {Position: insideC, Button: mouse.ButtonRelease}, + {Position: insideC, Button: mouse.ButtonLeft}, + {Position: insideC, Button: mouse.ButtonRelease}, }, - wantFocused: contLocRight, + wantFocused: contLocC, wantProcessed: 4, }, { desc: "click in the same container and release never happens", events: []*terminalapi.Mouse{ - {Position: insideRight, Button: mouse.ButtonLeft}, - {Position: insideLeft, Button: mouse.ButtonLeft}, - {Position: insideLeft, Button: mouse.ButtonRelease}, + {Position: insideC, Button: mouse.ButtonLeft}, + {Position: insideB, Button: mouse.ButtonLeft}, + {Position: insideB, Button: mouse.ButtonRelease}, }, - wantFocused: contLocLeft, + wantFocused: contLocB, wantProcessed: 3, }, { desc: "click in the same container, release elsewhere", events: []*terminalapi.Mouse{ - {Position: insideRight, Button: mouse.ButtonLeft}, - {Position: insideLeft, Button: mouse.ButtonRelease}, + {Position: insideC, Button: mouse.ButtonLeft}, + {Position: insideB, Button: mouse.ButtonRelease}, }, - wantFocused: contLocRoot, + wantFocused: contLocA, wantProcessed: 2, }, { desc: "other buttons are ignored", events: []*terminalapi.Mouse{ - {Position: insideLeft, Button: mouse.ButtonMiddle}, - {Position: insideLeft, Button: mouse.ButtonRelease}, - {Position: insideLeft, Button: mouse.ButtonRight}, - {Position: insideLeft, Button: mouse.ButtonRelease}, - {Position: insideLeft, Button: mouse.ButtonWheelUp}, - {Position: insideLeft, Button: mouse.ButtonWheelDown}, + {Position: insideB, Button: mouse.ButtonMiddle}, + {Position: insideB, Button: mouse.ButtonRelease}, + {Position: insideB, Button: mouse.ButtonRight}, + {Position: insideB, Button: mouse.ButtonRelease}, + {Position: insideB, Button: mouse.ButtonWheelUp}, + {Position: insideB, Button: mouse.ButtonWheelDown}, }, - wantFocused: contLocRoot, + wantFocused: contLocA, wantProcessed: 6, }, { @@ -375,27 +393,27 @@ func TestFocusTrackerMouse(t *testing.T) { {Position: image.Point{1, 1}, Button: mouse.ButtonLeft}, {Position: image.Point{2, 2}, Button: mouse.ButtonRelease}, }, - wantFocused: contLocLeft, + wantFocused: contLocB, wantProcessed: 3, }, { desc: "click ignored if followed by another click of the same button elsewhere", events: []*terminalapi.Mouse{ - {Position: insideRight, Button: mouse.ButtonLeft}, - {Position: insideLeft, Button: mouse.ButtonLeft}, - {Position: insideRight, Button: mouse.ButtonRelease}, + {Position: insideC, Button: mouse.ButtonLeft}, + {Position: insideB, Button: mouse.ButtonLeft}, + {Position: insideC, Button: mouse.ButtonRelease}, }, - wantFocused: contLocRoot, + wantFocused: contLocA, wantProcessed: 3, }, { desc: "click ignored if followed by another click of a different button", events: []*terminalapi.Mouse{ - {Position: insideRight, Button: mouse.ButtonLeft}, - {Position: insideRight, Button: mouse.ButtonMiddle}, - {Position: insideRight, Button: mouse.ButtonRelease}, + {Position: insideC, Button: mouse.ButtonLeft}, + {Position: insideC, Button: mouse.ButtonMiddle}, + {Position: insideC, Button: mouse.ButtonRelease}, }, - wantFocused: contLocRoot, + wantFocused: contLocA, wantProcessed: 3, }, } @@ -433,22 +451,22 @@ func TestFocusTrackerMouse(t *testing.T) { var wantFocused *Container switch wf := tc.wantFocused; wf { - case contLocRoot: + case contLocA: wantFocused = root - case contLocLeft: + case contLocB: wantFocused = root.first - case contLocRight: + case contLocC: wantFocused = root.second default: t.Fatalf("unsupported wantFocused value => %v", wf) } if !root.focusTracker.isActive(wantFocused) { - t.Errorf("isActive(%v) => false, want true, status: root(%v):%v, left(%v):%v, right(%v):%v", + t.Errorf("isActive(%v) => false, want true, status: contLocA(%v):%v, contLocB(%v):%v, contLocC(%v):%v", tc.wantFocused, - contLocRoot, root.focusTracker.isActive(root), - contLocLeft, root.focusTracker.isActive(root.first), - contLocRight, root.focusTracker.isActive(root.second), + contLocA, root.focusTracker.isActive(root), + contLocB, root.focusTracker.isActive(root.first), + contLocC, root.focusTracker.isActive(root.second), ) } }) @@ -479,6 +497,8 @@ const ( ) func TestFocusTrackerNextAndPrevious(t *testing.T) { + t.Log(contLocIntro()) + ft, err := faketerm.New(image.Point{10, 10}) if err != nil { t.Fatalf("faketerm.New => unexpected error: %v", err) @@ -508,7 +528,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { KeyFocusNext(keyNext), ) }, - wantFocused: contLocRoot, + wantFocused: contLocA, }, { desc: "keyNext does nothing when only root exists", @@ -521,7 +541,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { events: []*terminalapi.Keyboard{ {Key: keyNext}, }, - wantFocused: contLocRoot, + wantFocused: contLocA, wantProcessed: 1, }, { @@ -539,7 +559,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { events: []*terminalapi.Keyboard{ {Key: keyNext}, }, - wantFocused: contLocLeft, + wantFocused: contLocB, wantProcessed: 1, }, { @@ -558,7 +578,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { {Key: keyNext}, {Key: keyNext}, }, - wantFocused: contLocRight, + wantFocused: contLocC, wantProcessed: 2, }, { @@ -578,7 +598,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { {Key: keyNext}, {Key: keyNext}, }, - wantFocused: contLocLeft, + wantFocused: contLocB, wantProcessed: 3, }, { @@ -599,7 +619,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { {Key: keyNext}, {Key: keyNext}, }, - wantFocused: contLocRight, + wantFocused: contLocC, wantProcessed: 4, }, { @@ -621,7 +641,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { {Key: keyNext}, {Key: keyNext}, }, - wantFocused: contLocLeft, + wantFocused: contLocB, wantProcessed: 5, }, { @@ -635,7 +655,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { events: []*terminalapi.Keyboard{ {Key: keyPrevious}, }, - wantFocused: contLocRoot, + wantFocused: contLocA, wantProcessed: 1, }, { @@ -653,7 +673,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { events: []*terminalapi.Keyboard{ {Key: keyPrevious}, }, - wantFocused: contLocRight, + wantFocused: contLocC, wantProcessed: 1, }, { @@ -672,7 +692,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { {Key: keyPrevious}, {Key: keyPrevious}, }, - wantFocused: contLocLeft, + wantFocused: contLocB, wantProcessed: 2, }, { @@ -692,7 +712,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { {Key: keyPrevious}, {Key: keyPrevious}, }, - wantFocused: contLocRight, + wantFocused: contLocC, wantProcessed: 3, }, { @@ -713,7 +733,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { {Key: keyPrevious}, {Key: keyPrevious}, }, - wantFocused: contLocLeft, + wantFocused: contLocB, wantProcessed: 4, }, { @@ -735,7 +755,7 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { {Key: keyPrevious}, {Key: keyPrevious}, }, - wantFocused: contLocRight, + wantFocused: contLocC, wantProcessed: 5, }, } @@ -763,22 +783,22 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { var wantFocused *Container switch wf := tc.wantFocused; wf { - case contLocRoot: + case contLocA: wantFocused = root - case contLocLeft: + case contLocB: wantFocused = root.first - case contLocRight: + case contLocC: wantFocused = root.second default: t.Fatalf("unsupported wantFocused value => %v", wf) } if !root.focusTracker.isActive(wantFocused) { - t.Errorf("isActive(%v) => false, want true, status: root(%v):%v, left(%v):%v, right(%v):%v", + t.Errorf("isActive(%v) => false, want true, status: contLocA(%v):%v, contLocB(%v):%v, contLocC(%v):%v", tc.wantFocused, - contLocRoot, root.focusTracker.isActive(root), - contLocLeft, root.focusTracker.isActive(root.first), - contLocRight, root.focusTracker.isActive(root.second), + contLocA, root.focusTracker.isActive(root), + contLocB, root.focusTracker.isActive(root.first), + contLocC, root.focusTracker.isActive(root.second), ) } }) From 2f36dce2ed915abe90ccefcfbc4dbd31eeb03a24 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Sun, 29 Nov 2020 22:38:43 -0500 Subject: [PATCH 2/2] Containers can request to be skipped when focus is moved using keyboard keys. --- CHANGELOG.md | 3 + container/focus.go | 6 +- container/focus_test.go | 128 +++++++++++++++++++++++++++++++++++++++- container/options.go | 16 +++++ 4 files changed, 149 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0e6bd2e..5da0095 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,8 +31,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added #### Infrastructure changes + - ability to configure keyboard keys that move focus to the next or the previous container. +- containers can request to be skipped when focus is moved using keyboard keys. - widgets can now request keyboard events exclusively when focused. - ability to configure keyboard keys that move focus to the next or the previous container. @@ -40,6 +42,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the next or the previous container. #### Updates to the `button` widget + - the `button` widget allows users to specify multiple trigger keys. - the `button` widget now supports different keys for the global and focused scope. diff --git a/container/focus.go b/container/focus.go index a06f959..8e91673 100644 --- a/container/focus.go +++ b/container/focus.go @@ -92,7 +92,7 @@ func (ft *focusTracker) next() { return nil } - if c.isLeaf() && firstCont == nil { + if c.isLeaf() && !c.opts.keyFocusSkip && firstCont == nil { // Remember the first eligible container in case we "wrap" over, // i.e. finish the iteration before finding the next container. firstCont = c @@ -105,7 +105,7 @@ func (ft *focusTracker) next() { return nil } - if c.isLeaf() && focusNext { + if c.isLeaf() && !c.opts.keyFocusSkip && focusNext { nextCont = c } return nil @@ -133,7 +133,7 @@ func (ft *focusTracker) previous() { visitedCurr = true } - if c.isLeaf() { + if c.isLeaf() && !c.opts.keyFocusSkip { if !visitedCurr { // Remember the last eligible container closest to the one // currently focused. diff --git a/container/focus_test.go b/container/focus_test.go index cb5ab1c..faa07f1 100644 --- a/container/focus_test.go +++ b/container/focus_test.go @@ -265,7 +265,7 @@ func TestPointCont(t *testing.T) { // locations on test failures. func contLocIntro() string { var s strings.Builder - s.WriteString("Container locations refer to containers in the following three, i.e. contLocA is the root container:\n") + s.WriteString("Container locations refer to containers in the following tree, i.e. contLocA is the root container:\n") s.WriteString(` A / \ @@ -758,6 +758,132 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { wantFocused: contLocC, wantProcessed: 5, }, + { + desc: "first container requests to be skipped on key based focus changes, using next", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left( + KeyFocusSkip(), + ), + Right(), + ), + KeyFocusNext(keyNext), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyNext}, + }, + wantFocused: contLocC, + wantProcessed: 1, + }, + { + desc: "last container requests to be skipped on key based focus changes, using next", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right( + KeyFocusSkip(), + ), + ), + KeyFocusNext(keyNext), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyNext}, + {Key: keyNext}, + }, + wantFocused: contLocB, + wantProcessed: 2, + }, + { + desc: "all containers request to be skipped on key based focus changes, using next", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left( + KeyFocusSkip(), + ), + Right( + KeyFocusSkip(), + ), + ), + KeyFocusNext(keyNext), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyNext}, + }, + wantFocused: contLocA, + wantProcessed: 1, + }, + { + desc: "first container requests to be skipped on key based focus changes, using previous", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left( + KeyFocusSkip(), + ), + Right(), + ), + KeyFocusPrevious(keyPrevious), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyPrevious}, + {Key: keyPrevious}, + }, + wantFocused: contLocC, + wantProcessed: 2, + }, + { + desc: "last container requests to be skipped on key based focus changes, using previous", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right( + KeyFocusSkip(), + ), + ), + KeyFocusPrevious(keyPrevious), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyPrevious}, + }, + wantFocused: contLocB, + wantProcessed: 1, + }, + { + desc: "all containers request to be skipped on key based focus changes, using previous", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left( + KeyFocusSkip(), + ), + Right( + KeyFocusSkip(), + ), + ), + KeyFocusPrevious(keyPrevious), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyPrevious}, + }, + wantFocused: contLocA, + wantProcessed: 1, + }, } for _, tc := range tests { diff --git a/container/options.go b/container/options.go index 0ebf6f9..925fe31 100644 --- a/container/options.go +++ b/container/options.go @@ -132,6 +132,10 @@ type options struct { // margin is a space reserved on the outside of the container. margin margin + + // keyFocusSkip asserts whether this container should be skipped when focus + // is being moved using either of KeyFocusNext or KeyFocusPrevious. + keyFocusSkip bool } // margin stores the configured margin for the container. @@ -873,3 +877,15 @@ func KeyFocusPrevious(key keyboard.Key) Option { return nil }) } + +// KeyFocusSkip indicates that this container should never receive the keyboard +// focus when KeyFocusNext or KeyFocusPrevious is pressed. +// +// A container configured like this would still receive the keyboard focus when +// directly clicked on with a mouse. +func KeyFocusSkip() Option { + return option(func(c *Container) error { + c.opts.keyFocusSkip = true + return nil + }) +}