From 0771a92dc255ba0912a552742b66c66502c681f9 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Mon, 23 Nov 2020 22:33:24 -0500 Subject: [PATCH 1/7] Ability to move focus to the next container using a key. --- container/container.go | 25 ++++- container/focus.go | 43 ++++++++ container/focus_test.go | 217 ++++++++++++++++++++++++++++++++++++++++ container/options.go | 57 +++++++++++ 4 files changed, 338 insertions(+), 4 deletions(-) diff --git a/container/container.go b/container/container.go index 54cef78..ab60218 100644 --- a/container/container.go +++ b/container/container.go @@ -122,6 +122,12 @@ func (c *Container) hasWidget() bool { return c.opts.widget != nil } +// isLeaf determines if this container is a leaf container in the binary tree of containers. +// Only leaf containers are guaranteed to be "visible" on the screen. +func (c *Container) isLeaf() bool { + return c.first == nil && c.second == nil +} + // usable returns the usable area in this container. // This depends on whether the container has a border, etc. func (c *Container) usable() image.Rectangle { @@ -257,10 +263,10 @@ func (c *Container) Update(id string, opts ...Option) error { return nil } -// updateFocus processes the mouse event and determines if it changes the -// focused container. +// updateFocusFromMouse processes the mouse event and determines if it changes +// the focused container. // Caller must hold c.mu. -func (c *Container) updateFocus(m *terminalapi.Mouse) { +func (c *Container) updateFocusFromMouse(m *terminalapi.Mouse) { target := pointCont(c, m.Position) if target == nil { // Ignore mouse clicks where no containers are. return @@ -268,6 +274,15 @@ func (c *Container) updateFocus(m *terminalapi.Mouse) { c.focusTracker.mouse(target, m) } +// updateFocusFromKeyboard processes the keyboard event and determines if it +// changes the focused container. +// Caller must hold c.mu. +func (c *Container) updateFocusFromKeyboard(k *terminalapi.Keyboard) { + if c.opts.global.keyFocusNext != nil && *c.opts.global.keyFocusNext == k.Key { + c.focusTracker.next() + } +} + // processEvent processes events delivered to the container. func (c *Container) processEvent(ev terminalapi.Event) error { // This is done in two stages. @@ -293,7 +308,7 @@ func (c *Container) processEvent(ev terminalapi.Event) error { func (c *Container) prepareEvTargets(ev terminalapi.Event) (func() error, error) { switch e := ev.(type) { case *terminalapi.Mouse: - c.updateFocus(ev.(*terminalapi.Mouse)) + c.updateFocusFromMouse(ev.(*terminalapi.Mouse)) targets, err := c.mouseEvTargets(e) if err != nil { @@ -309,6 +324,8 @@ func (c *Container) prepareEvTargets(ev terminalapi.Event) (func() error, error) }, nil case *terminalapi.Keyboard: + c.updateFocusFromKeyboard(ev.(*terminalapi.Keyboard)) + targets := c.keyEvTargets() return func() error { for _, w := range targets { diff --git a/container/focus.go b/container/focus.go index 4320eea..6b0b229 100644 --- a/container/focus.go +++ b/container/focus.go @@ -78,6 +78,49 @@ func (ft *focusTracker) setActive(c *Container) { ft.container = c } +// next moves focus to the next container. +func (ft *focusTracker) next() { + var ( + errStr string + first *Container + cont *Container + focusNext bool + ) + postOrder(rootCont(ft.container), &errStr, visitFunc(func(c *Container) error { + if cont != nil { + // Already found the next container, nothing to do. + return nil + } + + if c.isLeaf() && first == nil { + // Remember the first eligible container in case we "wrap" over, + // i.e. finish the iteration before finding the next container. + first = c + } + + if ft.container == c { + // Visiting the currently focused container, going to focus the + // next one. + focusNext = true + return nil + } + + if c.isLeaf() && focusNext { + cont = c + } + return nil + })) + + if cont == nil && first != nil { + // If the traversal finishes without finding the next container, move + // focus back to the first container. + cont = first + } + if cont != nil { + ft.setActive(cont) + } +} + // mouse identifies mouse events that change the focused container and track // the focused container in the tree. // The argument c is the container onto which the mouse event landed. diff --git a/container/focus_test.go b/container/focus_test.go index 4a31ad7..451efde 100644 --- a/container/focus_test.go +++ b/container/focus_test.go @@ -21,6 +21,7 @@ import ( "time" "github.com/mum4k/termdash/cell" + "github.com/mum4k/termdash/keyboard" "github.com/mum4k/termdash/linestyle" "github.com/mum4k/termdash/mouse" "github.com/mum4k/termdash/private/event" @@ -453,3 +454,219 @@ func TestFocusTrackerMouse(t *testing.T) { }) } } + +// contDir represents a direction in which we want to change container focus. +type contDir int + +// String implements fmt.Stringer() +func (cd contDir) String() string { + if n, ok := contDirNames[cd]; ok { + return n + } + return "contDirUnknown" +} + +// contDirNames maps contDir values to human readable names. +var contDirNames = map[contDir]string{ + contDirNext: "contDirNext", + contDirPrevious: "contDirPrevious", +} + +const ( + contDirUnknown contDir = iota + contDirNext + contDirPrevious +) + +func TestFocusTrackerNextAndPrevious(t *testing.T) { + ft, err := faketerm.New(image.Point{10, 10}) + if err != nil { + t.Fatalf("faketerm.New => unexpected error: %v", err) + } + + const ( + keyNext keyboard.Key = keyboard.KeyTab + keyPrevious keyboard.Key = '~' + ) + + tests := []struct { + desc string + container func(ft *faketerm.Terminal) (*Container, error) + events []*terminalapi.Keyboard + wantFocused contLoc + wantProcessed int + }{ + { + desc: "initially the root is focused", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusNext(keyNext), + ) + }, + wantFocused: contLocRoot, + }, + { + desc: "keyNext does nothing when only root exists", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + KeyFocusNext(keyNext), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyNext}, + }, + wantFocused: contLocRoot, + wantProcessed: 1, + }, + { + desc: "keyNext focuses the first container", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusNext(keyNext), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyNext}, + }, + wantFocused: contLocLeft, + wantProcessed: 1, + }, + { + desc: "two keyNext presses focuses the second container", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusNext(keyNext), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyNext}, + {Key: keyNext}, + }, + wantFocused: contLocRight, + wantProcessed: 2, + }, + { + desc: "three keyNext presses focuses the first container again", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusNext(keyNext), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyNext}, + {Key: keyNext}, + {Key: keyNext}, + }, + wantFocused: contLocLeft, + wantProcessed: 3, + }, + { + desc: "four keyNext presses focuses the second container again", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusNext(keyNext), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyNext}, + {Key: keyNext}, + {Key: keyNext}, + {Key: keyNext}, + }, + wantFocused: contLocRight, + wantProcessed: 4, + }, + { + desc: "five keyNext presses focuses the first container again", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusNext(keyNext), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyNext}, + {Key: keyNext}, + {Key: keyNext}, + {Key: keyNext}, + {Key: keyNext}, + }, + wantFocused: contLocLeft, + wantProcessed: 5, + }, + } + + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + root, err := tc.container(ft) + if err != nil { + t.Fatalf("tc.container => unexpected error: %v", err) + } + + eds := event.NewDistributionSystem() + root.Subscribe(eds) + for _, ev := range tc.events { + eds.Event(ev) + } + if err := testevent.WaitFor(5*time.Second, func() error { + if got, want := eds.Processed(), tc.wantProcessed; got != want { + return fmt.Errorf("the event distribution system processed %d events, want %d", got, want) + } + return nil + }); err != nil { + t.Fatalf("testevent.WaitFor => %v", err) + } + + var wantFocused *Container + switch wf := tc.wantFocused; wf { + case contLocRoot: + wantFocused = root + case contLocLeft: + wantFocused = root.first + case contLocRight: + 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", + tc.wantFocused, + contLocRoot, root.focusTracker.isActive(root), + contLocLeft, root.focusTracker.isActive(root.first), + contLocRight, root.focusTracker.isActive(root.second), + ) + } + }) + } +} diff --git a/container/options.go b/container/options.go index 2d34af4..06019db 100644 --- a/container/options.go +++ b/container/options.go @@ -23,6 +23,7 @@ import ( "github.com/mum4k/termdash/align" "github.com/mum4k/termdash/cell" + "github.com/mum4k/termdash/keyboard" "github.com/mum4k/termdash/linestyle" "github.com/mum4k/termdash/private/area" "github.com/mum4k/termdash/widgetapi" @@ -95,7 +96,15 @@ type options struct { // id is the identifier provided by the user. id string + // global are options that apply globally to all containers in the tree. + // There is only one instance of these options in the entire tree, if any + // of the child containers change their values, the new values apply to the + // entire container tree. + global *globalOptions + // inherited are options that are inherited by child containers. + // After inheriting these options, the child container can set them to + // different values. inherited inherited // split identifies how is this container split. @@ -181,11 +190,23 @@ type inherited struct { focusedColor cell.Color } +// globalOptions are options that can only have a single value across the +// entire tree of containers. +// Regardless of which container they get set on, the new value will take +// effect on all the containers in the tree. +type globalOptions struct { + // keyFocusNext when set is the key that moves the focus to the next container. + keyFocusNext *keyboard.Key + // keyFocusPrevious when set is the key that moves the focus to the previous container. + keyFocusPrevious *keyboard.Key +} + // newOptions returns a new options instance with the default values. // Parent are the inherited options from the parent container or nil if these // options are for a container with no parent (the root). func newOptions(parent *options) *options { opts := &options{ + global: &globalOptions{}, inherited: inherited{ focusedColor: cell.ColorYellow, }, @@ -195,6 +216,7 @@ func newOptions(parent *options) *options { splitFixed: DefaultSplitFixed, } if parent != nil { + opts.global = parent.global opts.inherited = parent.inherited } return opts @@ -815,3 +837,38 @@ func Bottom(opts ...Option) BottomOption { return opts }) } + +// KeyFocusNext configures a key that moves the keyboard focus to the next +// container when pressed. +// +// Containers are organized in a binary tree, when the focus moves to the next +// container, it targets the next leaf container in a DFS traversal that +// contains a widget. Non-leaf containers and containers without widgets are +// skipped. If the currently focused container is the last container, the focus +// moves back to the first container. +// +// This option is global and applies to all created containers. +// If not specified, keyboard the focused container can only be changed by using the mouse. +func KeyFocusNext(key keyboard.Key) Option { + return option(func(c *Container) error { + c.opts.global.keyFocusNext = &key + return nil + }) +} + +// KeyFocusPrevious configures a key that moves the keyboard focus to the +// previous container when pressed. +// +// Containers are organized in a binary tree, when the focus moves to the previous +// container, it targets the previous leaf container in a DFS traversal that +// contains a widget. Non-leaf containers and containers without widgets are +// skipped. If the currently focused container is the first container, the focus +// moves back to the last container. +// +// This option is global and applies to all created containers. +func KeyFocusPrevious(key keyboard.Key) Option { + return option(func(c *Container) error { + c.opts.global.keyFocusPrevious = &key + return nil + }) +} From 0b94ed0f7c27ead0bc98fbf67be4ac0308024a9f Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Mon, 23 Nov 2020 22:35:08 -0500 Subject: [PATCH 2/7] Updating CHANGELOG. --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 29acdea..909e43e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- ability to configure keyboard keys that move focus to the next or the + previous container. + ## [0.13.0] - 17-Nov-2020 ### Added From e8565e739d329e0fd4e145175424d5f1e4305fd6 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Mon, 23 Nov 2020 22:42:29 -0500 Subject: [PATCH 3/7] Correcting documentation for the new options. --- container/options.go | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/container/options.go b/container/options.go index 06019db..c1308a0 100644 --- a/container/options.go +++ b/container/options.go @@ -842,13 +842,13 @@ func Bottom(opts ...Option) BottomOption { // container when pressed. // // Containers are organized in a binary tree, when the focus moves to the next -// container, it targets the next leaf container in a DFS traversal that -// contains a widget. Non-leaf containers and containers without widgets are -// skipped. If the currently focused container is the last container, the focus -// moves back to the first container. +// container, it targets the next leaf container in a DFS traversal. +// Non-leaf containers are skipped. If the currently focused container is the +// last container, the focus moves back to the first container. // // This option is global and applies to all created containers. -// If not specified, keyboard the focused container can only be changed by using the mouse. +// If neither of (KeyFocusNext, KeyFocusPrevious) is specified, the keyboard +// focus can only be changed by using the mouse. func KeyFocusNext(key keyboard.Key) Option { return option(func(c *Container) error { c.opts.global.keyFocusNext = &key @@ -860,12 +860,13 @@ func KeyFocusNext(key keyboard.Key) Option { // previous container when pressed. // // Containers are organized in a binary tree, when the focus moves to the previous -// container, it targets the previous leaf container in a DFS traversal that -// contains a widget. Non-leaf containers and containers without widgets are -// skipped. If the currently focused container is the first container, the focus -// moves back to the last container. +// container, it targets the previous leaf container in a DFS traversal. +// Non-leaf containers are skipped. If the currently focused container is the +// first container, the focus moves back to the last container. // // This option is global and applies to all created containers. +// If neither of (KeyFocusNext, KeyFocusPrevious) is specified, the keyboard +// focus can only be changed by using the mouse. func KeyFocusPrevious(key keyboard.Key) Option { return option(func(c *Container) error { c.opts.global.keyFocusPrevious = &key From 1fd4b044cfba3d2ac1ae1e682382ea0296474b58 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Mon, 23 Nov 2020 22:44:28 -0500 Subject: [PATCH 4/7] Clarifying the meaning of DFS. --- container/options.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/container/options.go b/container/options.go index c1308a0..0ebf6f9 100644 --- a/container/options.go +++ b/container/options.go @@ -842,7 +842,7 @@ func Bottom(opts ...Option) BottomOption { // container when pressed. // // Containers are organized in a binary tree, when the focus moves to the next -// container, it targets the next leaf container in a DFS traversal. +// container, it targets the next leaf container in a DFS (Depth-first search) traversal. // Non-leaf containers are skipped. If the currently focused container is the // last container, the focus moves back to the first container. // @@ -860,7 +860,7 @@ func KeyFocusNext(key keyboard.Key) Option { // previous container when pressed. // // Containers are organized in a binary tree, when the focus moves to the previous -// container, it targets the previous leaf container in a DFS traversal. +// container, it targets the previous leaf container in a DFS (Depth-first search) traversal. // Non-leaf containers are skipped. If the currently focused container is the // first container, the focus moves back to the last container. // From e4edc8f15a33a795622b77c3b8ce19241f26e334 Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 24 Nov 2020 14:24:08 -0500 Subject: [PATCH 5/7] Ability to focus the previous container using keyboard. --- container/container.go | 5 +- container/focus.go | 53 +++++++++++++++---- container/focus_test.go | 114 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 160 insertions(+), 12 deletions(-) diff --git a/container/container.go b/container/container.go index ab60218..ba69898 100644 --- a/container/container.go +++ b/container/container.go @@ -278,8 +278,11 @@ func (c *Container) updateFocusFromMouse(m *terminalapi.Mouse) { // changes the focused container. // Caller must hold c.mu. func (c *Container) updateFocusFromKeyboard(k *terminalapi.Keyboard) { - if c.opts.global.keyFocusNext != nil && *c.opts.global.keyFocusNext == k.Key { + switch { + case c.opts.global.keyFocusNext != nil && *c.opts.global.keyFocusNext == k.Key: c.focusTracker.next() + case c.opts.global.keyFocusPrevious != nil && *c.opts.global.keyFocusPrevious == k.Key: + c.focusTracker.previous() } } diff --git a/container/focus.go b/container/focus.go index 6b0b229..1e8c1f9 100644 --- a/container/focus.go +++ b/container/focus.go @@ -82,20 +82,20 @@ func (ft *focusTracker) setActive(c *Container) { func (ft *focusTracker) next() { var ( errStr string - first *Container - cont *Container + firstCont *Container + nextCont *Container focusNext bool ) - postOrder(rootCont(ft.container), &errStr, visitFunc(func(c *Container) error { - if cont != nil { + preOrder(rootCont(ft.container), &errStr, visitFunc(func(c *Container) error { + if nextCont != nil { // Already found the next container, nothing to do. return nil } - if c.isLeaf() && first == nil { + if c.isLeaf() && firstCont == nil { // Remember the first eligible container in case we "wrap" over, // i.e. finish the iteration before finding the next container. - first = c + firstCont = c } if ft.container == c { @@ -106,18 +106,49 @@ func (ft *focusTracker) next() { } if c.isLeaf() && focusNext { - cont = c + nextCont = c } return nil })) - if cont == nil && first != nil { + if nextCont == nil && firstCont != nil { // If the traversal finishes without finding the next container, move // focus back to the first container. - cont = first + nextCont = firstCont } - if cont != nil { - ft.setActive(cont) + if nextCont != nil { + ft.setActive(nextCont) + } +} + +// previous moves focus to the previous container. +func (ft *focusTracker) previous() { + var ( + errStr string + prevCont *Container + lastCont *Container + visitedCurr bool + ) + preOrder(rootCont(ft.container), &errStr, visitFunc(func(c *Container) error { + if ft.container == c { + visitedCurr = true + } + + if c.isLeaf() { + if !visitedCurr { + // Remember the last eligible container closest to the one + // currently focused. + prevCont = c + } + lastCont = c + } + return nil + })) + + if prevCont != nil { + ft.setActive(prevCont) + } else if lastCont != nil { + ft.setActive(lastCont) } } diff --git a/container/focus_test.go b/container/focus_test.go index 451efde..5f3a5a4 100644 --- a/container/focus_test.go +++ b/container/focus_test.go @@ -624,6 +624,120 @@ func TestFocusTrackerNextAndPrevious(t *testing.T) { wantFocused: contLocLeft, wantProcessed: 5, }, + { + desc: "keyPrevious does nothing when only root exists", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + KeyFocusPrevious(keyPrevious), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyPrevious}, + }, + wantFocused: contLocRoot, + wantProcessed: 1, + }, + { + desc: "keyPrevious focuses the last container", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusPrevious(keyPrevious), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyPrevious}, + }, + wantFocused: contLocRight, + wantProcessed: 1, + }, + { + desc: "two keyPrevious presses focuses the first container", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusPrevious(keyPrevious), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyPrevious}, + {Key: keyPrevious}, + }, + wantFocused: contLocLeft, + wantProcessed: 2, + }, + { + desc: "three keyPrevious presses focuses the second container again", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusPrevious(keyPrevious), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyPrevious}, + {Key: keyPrevious}, + {Key: keyPrevious}, + }, + wantFocused: contLocRight, + wantProcessed: 3, + }, + { + desc: "four keyPrevious presses focuses the first container again", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusPrevious(keyPrevious), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyPrevious}, + {Key: keyPrevious}, + {Key: keyPrevious}, + {Key: keyPrevious}, + }, + wantFocused: contLocLeft, + wantProcessed: 4, + }, + { + desc: "five keyPrevious presses focuses the second container again", + container: func(ft *faketerm.Terminal) (*Container, error) { + return New( + ft, + SplitVertical( + Left(), + Right(), + ), + KeyFocusPrevious(keyPrevious), + ) + }, + events: []*terminalapi.Keyboard{ + {Key: keyPrevious}, + {Key: keyPrevious}, + {Key: keyPrevious}, + {Key: keyPrevious}, + {Key: keyPrevious}, + }, + wantFocused: contLocRight, + wantProcessed: 5, + }, } for _, tc := range tests { From 765556fe8e9ea98dd24f149bbc155f709c10c05d Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 24 Nov 2020 15:18:50 -0500 Subject: [PATCH 6/7] faketerm diff: only print cells with differences. --- private/faketerm/diff.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/private/faketerm/diff.go b/private/faketerm/diff.go index 80b20e8..902a40a 100644 --- a/private/faketerm/diff.go +++ b/private/faketerm/diff.go @@ -100,6 +100,9 @@ func Diff(want, got *Terminal) string { for col := 0; col < size.X; col++ { got := got.BackBuffer()[col][row].Rune want := want.BackBuffer()[col][row].Rune + if got == want { + continue + } b.WriteString(fmt.Sprintf(" cell(%v, %v) => got '%c' (rune %d), want '%c' (rune %d)\n", col, row, got, got, want, want)) } } From ff431b782dfe9f438223fc789b4d187cd89b124e Mon Sep 17 00:00:00 2001 From: Jakub Sobon Date: Tue, 24 Nov 2020 15:19:14 -0500 Subject: [PATCH 7/7] e2e test cases for keyboard based focus changes. --- container/container.go | 8 +- container/container_test.go | 176 ++++++++++++++++++++++++++++++++++++ 2 files changed, 182 insertions(+), 2 deletions(-) diff --git a/container/container.go b/container/container.go index ba69898..242f8e8 100644 --- a/container/container.go +++ b/container/container.go @@ -327,9 +327,13 @@ func (c *Container) prepareEvTargets(ev terminalapi.Event) (func() error, error) }, nil case *terminalapi.Keyboard: - c.updateFocusFromKeyboard(ev.(*terminalapi.Keyboard)) - targets := c.keyEvTargets() + + // Update the focused container based on the pressed key. + // Done after collecting "targets" above. If the key changes which + // widget is focused, they key press itself should go to the widget + // that was focused when the key was pressed. + c.updateFocusFromKeyboard(ev.(*terminalapi.Keyboard)) return func() error { for _, w := range targets { if err := w.Keyboard(e); err != nil { diff --git a/container/container_test.go b/container/container_test.go index 1fd2c6d..f584c7e 100644 --- a/container/container_test.go +++ b/container/container_test.go @@ -1510,6 +1510,182 @@ func TestMouse(t *testing.T) { return ft }, }, + { + desc: "key event focuses the next container, widget with KeyScopeFocused does not get the key as it was not focused yet", + termSize: image.Point{50, 20}, + container: func(ft *faketerm.Terminal) (*Container, error) { + c, err := New( + ft, + SplitVertical( + Left( + PlaceWidget(fakewidget.New(widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused})), + ), + Right( + PlaceWidget(fakewidget.New(widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused})), + ), + ), + KeyFocusNext(keyboard.KeyTab), + ) + if err != nil { + return nil, err + } + return c, nil + }, + events: []terminalapi.Event{ + &terminalapi.Keyboard{Key: keyboard.KeyTab}, + }, + want: func(size image.Point) *faketerm.Terminal { + ft := faketerm.MustNew(size) + + fakewidget.MustDraw( + ft, + testcanvas.MustNew(image.Rect(0, 0, 25, 20)), + &widgetapi.Meta{Focused: true}, + widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused}, + ) + fakewidget.MustDraw( + ft, + testcanvas.MustNew(image.Rect(25, 0, 50, 20)), + &widgetapi.Meta{}, + widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused}, + &terminalapi.Keyboard{}, + ) + return ft + }, + }, + { + desc: "key event focuses the previous container, option set on both parent and child, the last option is used since focus keys are global options", + termSize: image.Point{50, 20}, + container: func(ft *faketerm.Terminal) (*Container, error) { + c, err := New( + ft, + KeyFocusPrevious(keyboard.KeyEnter), + SplitVertical( + Left( + PlaceWidget(fakewidget.New(widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused})), + ), + Right( + PlaceWidget(fakewidget.New(widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused})), + KeyFocusPrevious(keyboard.KeyTab), + ), + ), + ) + if err != nil { + return nil, err + } + return c, nil + }, + events: []terminalapi.Event{ + &terminalapi.Keyboard{Key: keyboard.KeyTab}, + }, + want: func(size image.Point) *faketerm.Terminal { + ft := faketerm.MustNew(size) + + fakewidget.MustDraw( + ft, + testcanvas.MustNew(image.Rect(0, 0, 25, 20)), + &widgetapi.Meta{}, + widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused}, + ) + fakewidget.MustDraw( + ft, + testcanvas.MustNew(image.Rect(25, 0, 50, 20)), + &widgetapi.Meta{Focused: true}, + widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused}, + &terminalapi.Keyboard{}, + ) + return ft + }, + }, + { + desc: "key event focuses the next container, widget with KeyScopeGlobal also gets the key", + termSize: image.Point{50, 20}, + container: func(ft *faketerm.Terminal) (*Container, error) { + c, err := New( + ft, + SplitVertical( + Left( + PlaceWidget(fakewidget.New(widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused})), + ), + Right( + PlaceWidget(fakewidget.New(widgetapi.Options{WantKeyboard: widgetapi.KeyScopeGlobal})), + ), + ), + KeyFocusNext(keyboard.KeyTab), + ) + if err != nil { + return nil, err + } + return c, nil + }, + events: []terminalapi.Event{ + &terminalapi.Keyboard{Key: keyboard.KeyTab}, + }, + want: func(size image.Point) *faketerm.Terminal { + ft := faketerm.MustNew(size) + + fakewidget.MustDraw( + ft, + testcanvas.MustNew(image.Rect(0, 0, 25, 20)), + &widgetapi.Meta{Focused: true}, + widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused}, + ) + fakewidget.MustDraw( + ft, + testcanvas.MustNew(image.Rect(25, 0, 50, 20)), + &widgetapi.Meta{}, + widgetapi.Options{WantKeyboard: widgetapi.KeyScopeGlobal}, + &terminalapi.Keyboard{Key: keyboard.KeyTab}, + ) + return ft + }, + }, + { + desc: "key event moves focus from a widget with KeyScopeFocused, the originally focused widget gets the key", + termSize: image.Point{50, 20}, + container: func(ft *faketerm.Terminal) (*Container, error) { + c, err := New( + ft, + SplitVertical( + Left( + PlaceWidget(fakewidget.New(widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused})), + ), + Right( + PlaceWidget(fakewidget.New(widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused})), + ), + ), + KeyFocusNext(keyboard.KeyTab), + ) + if err != nil { + return nil, err + } + return c, nil + }, + events: []terminalapi.Event{ + // Focus the left container. + &terminalapi.Keyboard{Key: keyboard.KeyTab}, + // Move focus from left to right. + &terminalapi.Keyboard{Key: keyboard.KeyTab}, + }, + want: func(size image.Point) *faketerm.Terminal { + ft := faketerm.MustNew(size) + + fakewidget.MustDraw( + ft, + testcanvas.MustNew(image.Rect(0, 0, 25, 20)), + &widgetapi.Meta{}, + widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused}, + &terminalapi.Keyboard{Key: keyboard.KeyTab}, + ) + fakewidget.MustDraw( + ft, + testcanvas.MustNew(image.Rect(25, 0, 50, 20)), + &widgetapi.Meta{Focused: true}, + widgetapi.Options{WantKeyboard: widgetapi.KeyScopeFocused}, + ) + return ft + }, + }, { desc: "event not forwarded if the widget didn't request it", termSize: image.Point{20, 20},