From b2ef44aec724ad16e1fe16d0c28c7439df568c59 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilo=20Pr=C3=BCtz?= Date: Tue, 8 Sep 2020 11:42:25 +0200 Subject: [PATCH 1/7] sort non-exported functions of glfw/canvas --- internal/driver/glfw/canvas.go | 226 ++++++++++++++++----------------- 1 file changed, 113 insertions(+), 113 deletions(-) diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index f5048bb857..38d4cf5836 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -310,14 +310,43 @@ func (c *glCanvas) AddShortcut(shortcut fyne.Shortcut, handler func(shortcut fyn c.shortcut.AddShortcut(shortcut, handler) } -func (c *glCanvas) objectTrees() []fyne.CanvasObject { - trees := make([]fyne.CanvasObject, 0, len(c.Overlays().List())+2) - trees = append(trees, c.content) - if c.menu != nil { - trees = append(trees, c.menu) +func (c *glCanvas) buildMenu(w *window, m *fyne.MainMenu) { + c.setMenuOverlay(nil) + if m == nil { + return } - trees = append(trees, c.Overlays().List()...) - return trees + if hasNativeMenu() { + setupNativeMenu(w, m) + } else { + c.setMenuOverlay(buildMenuOverlay(m, c)) + } +} + +// canvasSize computes the needed canvas size for the given content size +func (c *glCanvas) canvasSize(contentSize fyne.Size) fyne.Size { + canvasSize := contentSize.Add(fyne.NewSize(0, c.menuHeight())) + if c.Padded() { + pad := theme.Padding() * 2 + canvasSize = canvasSize.Add(fyne.NewSize(pad, pad)) + } + return canvasSize +} + +func (c *glCanvas) contentPos() fyne.Position { + contentPos := fyne.NewPos(0, c.menuHeight()) + if c.Padded() { + contentPos = contentPos.Add(fyne.NewPos(theme.Padding(), theme.Padding())) + } + return contentPos +} + +func (c *glCanvas) contentSize(canvasSize fyne.Size) fyne.Size { + contentSize := fyne.NewSize(canvasSize.Width, canvasSize.Height-c.menuHeight()) + if c.Padded() { + pad := theme.Padding() * 2 + contentSize = contentSize.Subtract(fyne.NewSize(pad, pad)) + } + return contentSize } func (c *glCanvas) ensureMinSize() bool { @@ -377,15 +406,35 @@ func (c *glCanvas) ensureMinSize() bool { return windowNeedsMinSizeUpdate } -func updateLayout(objToLayout fyne.CanvasObject) { - switch cont := objToLayout.(type) { - case *fyne.Container: - if cont.Layout != nil { - cont.Layout.Layout(cont.Objects, cont.Size()) - } - case fyne.Widget: - cache.Renderer(cont).Layout(cont.Size()) +func (c *glCanvas) isDirty() bool { + c.dirtyMutex.Lock() + defer c.dirtyMutex.Unlock() + + return c.dirty +} + +func (c *glCanvas) menuHeight() int { + switch c.menuOverlay() { + case nil: + // no menu or native menu -> does not consume space on the canvas + return 0 + default: + return c.menuOverlay().MinSize().Height + } +} + +func (c *glCanvas) menuOverlay() fyne.CanvasObject { + return c.menu +} + +func (c *glCanvas) objectTrees() []fyne.CanvasObject { + trees := make([]fyne.CanvasObject, 0, len(c.Overlays().List())+2) + trees = append(trees, c.content) + if c.menu != nil { + trees = append(trees, c.menu) } + trees = append(trees, c.Overlays().List()...) + return trees } func (c *glCanvas) paint(size fyne.Size) { @@ -415,19 +464,33 @@ func (c *glCanvas) paint(size fyne.Size) { c.walkTrees(paint, afterPaint) } -func (c *glCanvas) walkTrees( - beforeChildren func(*renderCacheNode, fyne.Position), - afterChildren func(*renderCacheNode), -) { - c.walkTree(c.contentTree, beforeChildren, afterChildren) - if c.menu != nil { - c.walkTree(c.menuTree, beforeChildren, afterChildren) - } - for _, tree := range c.overlays.renderCaches { - if tree != nil { - c.walkTree(tree, beforeChildren, afterChildren) +func (c *glCanvas) setDirty(dirty bool) { + c.dirtyMutex.Lock() + defer c.dirtyMutex.Unlock() + + c.dirty = dirty +} + +func (c *glCanvas) setMenuOverlay(b fyne.CanvasObject) { + c.Lock() + c.menu = b + c.menuTree = &renderCacheTree{root: &renderCacheNode{obj: c.menu}} + c.Unlock() +} + +func (c *glCanvas) setupThemeListener() { + listener := make(chan fyne.Settings) + fyne.CurrentApp().Settings().AddChangeListener(listener) + go func() { + for { + <-listener + if c.menu != nil { + app.ApplyThemeTo(c.menu, c) // Ensure our menu gets the theme change message as it's out-of-tree + } + + c.SetPadded(c.padded) // refresh the padding for potential theme differences } - } + }() } func (c *glCanvas) walkTree( @@ -484,93 +547,19 @@ func (c *glCanvas) walkTree( driver.WalkVisibleObjectTree(tree.root.obj, bc, ac) } -func (c *glCanvas) setDirty(dirty bool) { - c.dirtyMutex.Lock() - defer c.dirtyMutex.Unlock() - - c.dirty = dirty -} - -func (c *glCanvas) isDirty() bool { - c.dirtyMutex.Lock() - defer c.dirtyMutex.Unlock() - - return c.dirty -} - -func (c *glCanvas) setupThemeListener() { - listener := make(chan fyne.Settings) - fyne.CurrentApp().Settings().AddChangeListener(listener) - go func() { - for { - <-listener - if c.menu != nil { - app.ApplyThemeTo(c.menu, c) // Ensure our menu gets the theme change message as it's out-of-tree - } - - c.SetPadded(c.padded) // refresh the padding for potential theme differences - } - }() -} - -func (c *glCanvas) buildMenu(w *window, m *fyne.MainMenu) { - c.setMenuOverlay(nil) - if m == nil { - return - } - if hasNativeMenu() { - setupNativeMenu(w, m) - } else { - c.setMenuOverlay(buildMenuOverlay(m, c)) - } -} - -func (c *glCanvas) setMenuOverlay(b fyne.CanvasObject) { - c.Lock() - c.menu = b - c.menuTree = &renderCacheTree{root: &renderCacheNode{obj: c.menu}} - c.Unlock() -} - -func (c *glCanvas) menuOverlay() fyne.CanvasObject { - return c.menu -} - -func (c *glCanvas) menuHeight() int { - switch c.menuOverlay() { - case nil: - // no menu or native menu -> does not consume space on the canvas - return 0 - default: - return c.menuOverlay().MinSize().Height - } -} - -// canvasSize computes the needed canvas size for the given content size -func (c *glCanvas) canvasSize(contentSize fyne.Size) fyne.Size { - canvasSize := contentSize.Add(fyne.NewSize(0, c.menuHeight())) - if c.Padded() { - pad := theme.Padding() * 2 - canvasSize = canvasSize.Add(fyne.NewSize(pad, pad)) - } - return canvasSize -} - -func (c *glCanvas) contentSize(canvasSize fyne.Size) fyne.Size { - contentSize := fyne.NewSize(canvasSize.Width, canvasSize.Height-c.menuHeight()) - if c.Padded() { - pad := theme.Padding() * 2 - contentSize = contentSize.Subtract(fyne.NewSize(pad, pad)) +func (c *glCanvas) walkTrees( + beforeChildren func(*renderCacheNode, fyne.Position), + afterChildren func(*renderCacheNode), +) { + c.walkTree(c.contentTree, beforeChildren, afterChildren) + if c.menu != nil { + c.walkTree(c.menuTree, beforeChildren, afterChildren) } - return contentSize -} - -func (c *glCanvas) contentPos() fyne.Position { - contentPos := fyne.NewPos(0, c.menuHeight()) - if c.Padded() { - contentPos = contentPos.Add(fyne.NewPos(theme.Padding(), theme.Padding())) + for _, tree := range c.overlays.renderCaches { + if tree != nil { + c.walkTree(tree, beforeChildren, afterChildren) + } } - return contentPos } func newCanvas() *glCanvas { @@ -581,7 +570,7 @@ func newCanvas() *glCanvas { c.overlays = &overlayStack{onChange: func() { c.setDirty(true) }} - c.focusMgr = app.NewFocusManager(c) + c.focusMgr = app.NewFocusManager(c.content) c.refreshQueue = make(chan fyne.CanvasObject, 4096) c.dirtyMutex = &sync.Mutex{} @@ -589,3 +578,14 @@ func newCanvas() *glCanvas { return c } + +func updateLayout(objToLayout fyne.CanvasObject) { + switch cont := objToLayout.(type) { + case *fyne.Container: + if cont.Layout != nil { + cont.Layout.Layout(cont.Objects, cont.Size()) + } + case fyne.Widget: + cache.Renderer(cont).Layout(cont.Size()) + } +} From 6083e148287ee8748124eaa04bbc1b114f4bcc5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilo=20Pr=C3=BCtz?= Date: Tue, 8 Sep 2020 11:45:33 +0200 Subject: [PATCH 2/7] use Max instead of Union in glfw/canvas --- internal/driver/glfw/canvas.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 38d4cf5836..56298edb51 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -110,7 +110,7 @@ func (c *glCanvas) SetContent(content fyne.CanvasObject) { c.content.Resize(c.content.MinSize()) // give it the space it wants then calculate the real min // the pass above makes some layouts wide enough to wrap, so we ask again what the true min is. - newSize := c.size.Union(c.canvasSize(c.content.MinSize())) + newSize := c.size.Max(c.canvasSize(c.content.MinSize())) c.Unlock() c.Resize(newSize) @@ -375,7 +375,7 @@ func (c *glCanvas) ensureMinSize() bool { } else { windowNeedsMinSizeUpdate = true size := obj.Size() - expectedSize := minSize.Union(size) + expectedSize := minSize.Max(size) if expectedSize != size && size != c.size { objToLayout = nil obj.Resize(expectedSize) @@ -395,7 +395,7 @@ func (c *glCanvas) ensureMinSize() bool { shouldResize := windowNeedsMinSizeUpdate && (c.size.Width < min.Width || c.size.Height < min.Height) c.RUnlock() if shouldResize { - c.Resize(c.Size().Union(c.MinSize())) + c.Resize(c.Size().Max(c.MinSize())) } if lastParent != nil { From 682502991376fdc0b7952b6a74ba26baa800b34d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilo=20Pr=C3=BCtz?= Date: Tue, 8 Sep 2020 12:10:56 +0200 Subject: [PATCH 3/7] remove unnecessary `!found` from FocusManager --- internal/app/focus.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/app/focus.go b/internal/app/focus.go index ec0cd937ec..c6208f2a9e 100644 --- a/internal/app/focus.go +++ b/internal/app/focus.go @@ -29,7 +29,7 @@ func (f *FocusManager) nextInChain(current fyne.Focusable) fyne.Focusable { return true } - if !found && obj == current.(fyne.CanvasObject) { + if obj == current.(fyne.CanvasObject) { found = true } if first == nil { From 5300c5be93d325afb6b165828abd5c4dbea9392b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilo=20Pr=C3=BCtz?= Date: Tue, 8 Sep 2020 12:13:19 +0200 Subject: [PATCH 4/7] move focus handling from canvas to FocusManager The FocusManager no longer knows about the canvas. Instead the canvas creates a new FocusManager when its content is set. The canvas accesses its FocusManager to focus/unfocus objects. --- internal/app/focus.go | 63 +++++++++++++++++++++---------- internal/app/focus_test.go | 69 ++++++++++++++++++++++------------ internal/driver/glfw/canvas.go | 46 +++++++++++++++-------- internal/driver/glfw/window.go | 15 ++++---- 4 files changed, 128 insertions(+), 65 deletions(-) diff --git a/internal/app/focus.go b/internal/app/focus.go index c6208f2a9e..5f2bd39866 100644 --- a/internal/app/focus.go +++ b/internal/app/focus.go @@ -1,19 +1,59 @@ package app import ( + "sync" + "fyne.io/fyne" "fyne.io/fyne/internal/driver" ) // FocusManager represents a standard manager of input focus for a canvas type FocusManager struct { - canvas fyne.Canvas + sync.RWMutex + + content fyne.CanvasObject + focused fyne.Focusable +} + +// NewFocusManager returns a new instance of the standard focus manager for a canvas. +func NewFocusManager(c fyne.CanvasObject) *FocusManager { + return &FocusManager{content: c} +} + +// Focus focuses the given obj. +func (f *FocusManager) Focus(obj fyne.Focusable) { + f.Lock() + defer f.Unlock() + f.focused = obj +} + +// Focused returns the currently focused object or nil if none. +func (f *FocusManager) Focused() fyne.Focusable { + f.RLock() + defer f.RUnlock() + return f.focused +} + +// FocusNext will find the item after the current that can be focused and focus it. +// If current is nil then the first focusable item in the canvas will be focused. +func (f *FocusManager) FocusNext() { + f.Lock() + defer f.Unlock() + f.focused = f.nextInChain(f.focused) +} + +// FocusPrevious will find the item before the current that can be focused and focus it. +// If current is nil then the last focusable item in the canvas will be focused. +func (f *FocusManager) FocusPrevious() { + f.Lock() + defer f.Unlock() + f.focused = f.previousInChain(f.focused) } func (f *FocusManager) nextInChain(current fyne.Focusable) fyne.Focusable { var first, next fyne.Focusable found := current == nil // if we have no starting point then pretend we matched already - driver.WalkVisibleObjectTree(f.canvas.Content(), func(obj fyne.CanvasObject, _ fyne.Position, _ fyne.Position, _ fyne.Size) bool { + driver.WalkVisibleObjectTree(f.content, func(obj fyne.CanvasObject, _ fyne.Position, _ fyne.Position, _ fyne.Size) bool { if w, ok := obj.(fyne.Disableable); ok && w.Disabled() { // disabled widget cannot receive focus return false @@ -48,7 +88,7 @@ func (f *FocusManager) nextInChain(current fyne.Focusable) fyne.Focusable { func (f *FocusManager) previousInChain(current fyne.Focusable) fyne.Focusable { var last, previous fyne.Focusable found := false - driver.WalkVisibleObjectTree(f.canvas.Content(), func(obj fyne.CanvasObject, _ fyne.Position, _ fyne.Position, _ fyne.Size) bool { + driver.WalkVisibleObjectTree(f.content, func(obj fyne.CanvasObject, _ fyne.Position, _ fyne.Position, _ fyne.Size) bool { if w, ok := obj.(fyne.Disableable); ok && w.Disabled() { // disabled widget cannot receive focus return false @@ -76,20 +116,3 @@ func (f *FocusManager) previousInChain(current fyne.Focusable) fyne.Focusable { } return last } - -// FocusNext will find the item after the current that can be focused and focus it. -// If current is nil then the first focusable item in the canvas will be focused. -func (f *FocusManager) FocusNext(current fyne.Focusable) { - f.canvas.Focus(f.nextInChain(current)) -} - -// FocusPrevious will find the item before the current that can be focused and focus it. -// If current is nil then the last focusable item in the canvas will be focused. -func (f *FocusManager) FocusPrevious(current fyne.Focusable) { - f.canvas.Focus(f.previousInChain(current)) -} - -// NewFocusManager returns a new instance of the standard focus manager for a canvas. -func NewFocusManager(c fyne.Canvas) *FocusManager { - return &FocusManager{canvas: c} -} diff --git a/internal/app/focus_test.go b/internal/app/focus_test.go index 00baaac65d..b812b837c7 100644 --- a/internal/app/focus_test.go +++ b/internal/app/focus_test.go @@ -4,14 +4,38 @@ import ( "testing" "fyne.io/fyne/internal/app" - "fyne.io/fyne/test" "fyne.io/fyne/widget" "github.com/stretchr/testify/assert" ) +func TestFocusManager_Focus(t *testing.T) { + entry1 := widget.NewEntry() + hidden := widget.NewCheck("test", func(bool) {}) + hidden.Hide() + entry2 := widget.NewEntry() + disabled := widget.NewCheck("test", func(bool) {}) + disabled.Disable() + entry3 := widget.NewEntry() + c := widget.NewVBox(entry1, hidden, entry2, disabled, entry3) + + manager := app.NewFocusManager(c) + assert.Nil(t, manager.Focused()) + + manager.Focus(entry2) + assert.Equal(t, entry2, manager.Focused()) + + manager.Focus(entry1) + assert.Equal(t, entry1, manager.Focused()) + + manager.Focus(entry3) + assert.Equal(t, entry3, manager.Focused()) + + manager.Focus(nil) + assert.Nil(t, manager.Focused()) +} + func TestFocusManager_FocusNext(t *testing.T) { - c := test.NewCanvas() entry1 := widget.NewEntry() hidden := widget.NewCheck("test", func(bool) {}) hidden.Hide() @@ -19,26 +43,25 @@ func TestFocusManager_FocusNext(t *testing.T) { disabled := widget.NewCheck("test", func(bool) {}) disabled.Disable() entry3 := widget.NewEntry() - c.SetContent(widget.NewVBox(entry1, hidden, entry2, disabled, entry3)) + c := widget.NewVBox(entry1, hidden, entry2, disabled, entry3) manager := app.NewFocusManager(c) - assert.Nil(t, c.Focused()) + assert.Nil(t, manager.Focused()) - manager.FocusNext(nil) - assert.Equal(t, entry1, c.Focused()) + manager.FocusNext() + assert.Equal(t, entry1, manager.Focused()) - manager.FocusNext(entry1) - assert.Equal(t, entry2, c.Focused()) + manager.FocusNext() + assert.Equal(t, entry2, manager.Focused()) - manager.FocusNext(entry2) - assert.Equal(t, entry3, c.Focused()) + manager.FocusNext() + assert.Equal(t, entry3, manager.Focused()) - manager.FocusNext(entry3) - assert.Equal(t, entry1, c.Focused()) + manager.FocusNext() + assert.Equal(t, entry1, manager.Focused()) } func TestFocusManager_FocusPrevious(t *testing.T) { - c := test.NewCanvas() entry1 := widget.NewEntry() hidden := widget.NewCheck("test", func(bool) {}) hidden.Hide() @@ -46,20 +69,20 @@ func TestFocusManager_FocusPrevious(t *testing.T) { disabled := widget.NewCheck("test", func(bool) {}) disabled.Disable() entry3 := widget.NewEntry() - c.SetContent(widget.NewVBox(entry1, hidden, entry2, disabled, entry3)) + c := widget.NewVBox(entry1, hidden, entry2, disabled, entry3) manager := app.NewFocusManager(c) - assert.Nil(t, c.Focused()) + assert.Nil(t, manager.Focused()) - manager.FocusPrevious(nil) - assert.Equal(t, entry3, c.Focused()) + manager.FocusPrevious() + assert.Equal(t, entry3, manager.Focused()) - manager.FocusPrevious(entry3) - assert.Equal(t, entry2, c.Focused()) + manager.FocusPrevious() + assert.Equal(t, entry2, manager.Focused()) - manager.FocusPrevious(entry2) - assert.Equal(t, entry1, c.Focused()) + manager.FocusPrevious() + assert.Equal(t, entry1, manager.Focused()) - manager.FocusPrevious(entry1) - assert.Equal(t, entry3, c.Focused()) + manager.FocusPrevious() + assert.Equal(t, entry3, manager.Focused()) } diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 56298edb51..60b824b531 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -27,7 +27,6 @@ type glCanvas struct { overlays *overlayStack padded bool size fyne.Size - focused fyne.Focusable focusMgr *app.FocusManager onTypedRune func(rune) @@ -105,8 +104,7 @@ func (c *glCanvas) Content() fyne.CanvasObject { func (c *glCanvas) SetContent(content fyne.CanvasObject) { c.Lock() - c.content = content - c.contentTree = &renderCacheTree{root: &renderCacheNode{obj: c.content}} + c.setContent(content) c.content.Resize(c.content.MinSize()) // give it the space it wants then calculate the real min // the pass above makes some layouts wide enough to wrap, so we ask again what the true min is. @@ -165,8 +163,9 @@ func (c *glCanvas) Refresh(obj fyne.CanvasObject) { func (c *glCanvas) Focus(obj fyne.Focusable) { c.RLock() - focused := c.focused + mgr := c.focusMgr c.RUnlock() + focused := mgr.Focused() if focused == obj || obj == nil { return @@ -181,19 +180,32 @@ func (c *glCanvas) Focus(obj fyne.Focusable) { focused.FocusLost() } - c.Lock() - c.focused = obj - c.Unlock() + mgr.Focus(obj) obj.FocusGained() } +func (c *glCanvas) FocusNext() { + c.RLock() + mgr := c.focusMgr + c.RUnlock() + mgr.FocusNext() +} + +func (c *glCanvas) FocusPrevious() { + c.RLock() + mgr := c.focusMgr + c.RUnlock() + mgr.FocusPrevious() +} + func (c *glCanvas) Unfocus() { - c.Lock() - focused := c.focused - c.focused = nil - c.Unlock() + c.RLock() + mgr := c.focusMgr + c.RUnlock() + focused := mgr.Focused() if focused != nil { + mgr.Focus(nil) focused.FocusLost() } } @@ -201,7 +213,7 @@ func (c *glCanvas) Unfocus() { func (c *glCanvas) Focused() fyne.Focusable { c.RLock() defer c.RUnlock() - return c.focused + return c.focusMgr.Focused() } func (c *glCanvas) Resize(size fyne.Size) { @@ -464,6 +476,12 @@ func (c *glCanvas) paint(size fyne.Size) { c.walkTrees(paint, afterPaint) } +func (c *glCanvas) setContent(content fyne.CanvasObject) { + c.content = content + c.contentTree = &renderCacheTree{root: &renderCacheNode{obj: c.content}} + c.focusMgr = app.NewFocusManager(c.content) +} + func (c *glCanvas) setDirty(dirty bool) { c.dirtyMutex.Lock() defer c.dirtyMutex.Unlock() @@ -564,13 +582,11 @@ func (c *glCanvas) walkTrees( func newCanvas() *glCanvas { c := &glCanvas{scale: 1.0, texScale: 1.0} - c.content = &canvas.Rectangle{FillColor: theme.BackgroundColor()} - c.contentTree = &renderCacheTree{root: &renderCacheNode{obj: c.content}} + c.setContent(&canvas.Rectangle{FillColor: theme.BackgroundColor()}) c.padded = true c.overlays = &overlayStack{onChange: func() { c.setDirty(true) }} - c.focusMgr = app.NewFocusManager(c.content) c.refreshQueue = make(chan fyne.CanvasObject, 4096) c.dirtyMutex = &sync.Mutex{} diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index 57c0f1b671..f69d297ac8 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -906,12 +906,12 @@ func (w *window) keyPressed(_ *glfw.Window, key glfw.Key, scancode int, action g if keyName == fyne.KeyTab { if keyDesktopModifier == 0 { if action != glfw.Release { - w.canvas.focusMgr.FocusNext(w.canvas.focused) + w.canvas.FocusNext() } return } else if keyDesktopModifier == desktop.ShiftModifier { if action != glfw.Release { - w.canvas.focusMgr.FocusPrevious(w.canvas.focused) + w.canvas.FocusPrevious() } return } @@ -1038,15 +1038,16 @@ func (w *window) charInput(_ *glfw.Window, char rune) { } } -func (w *window) focused(_ *glfw.Window, focused bool) { - if w.canvas.focused == nil { +func (w *window) focused(_ *glfw.Window, isFocused bool) { + focused := w.canvas.Focused() + if focused == nil { return } - if focused { - w.canvas.focused.FocusGained() + if isFocused { + focused.FocusGained() } else { - w.canvas.focused.FocusLost() + focused.FocusLost() } } From 8d380dc19a0cdfec310d252265cd5c2c6fe262f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilo=20Pr=C3=BCtz?= Date: Tue, 8 Sep 2020 12:20:55 +0200 Subject: [PATCH 5/7] add missing space in comment --- internal/driver/glfw/window.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index f69d297ac8..7efe189252 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -430,7 +430,7 @@ func (w *window) ShowAndRun() { fyne.CurrentApp().Driver().Run() } -//Clipboard returns the system clipboard +// Clipboard returns the system clipboard func (w *window) Clipboard() fyne.Clipboard { if w.viewport == nil { return nil From a3be02ba0aa90c1667915f07289bab20ebff2d79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilo=20Pr=C3=BCtz?= Date: Sat, 12 Sep 2020 08:36:38 +0200 Subject: [PATCH 6/7] move focus gained/lost notification from canvas to FocusManager --- internal/app/focus.go | 15 ++++++++++ internal/driver/glfw/canvas.go | 51 ++++++++-------------------------- 2 files changed, 26 insertions(+), 40 deletions(-) diff --git a/internal/app/focus.go b/internal/app/focus.go index 5f2bd39866..80a4d4e6b4 100644 --- a/internal/app/focus.go +++ b/internal/app/focus.go @@ -24,7 +24,22 @@ func NewFocusManager(c fyne.CanvasObject) *FocusManager { func (f *FocusManager) Focus(obj fyne.Focusable) { f.Lock() defer f.Unlock() + + if f.focused == obj { + return + } + + if dis, ok := obj.(fyne.Disableable); ok && dis.Disabled() { + obj = nil + } + + if f.focused != nil { + f.focused.FocusLost() + } f.focused = obj + if obj != nil { + obj.FocusGained() + } } // Focused returns the currently focused object or nil if none. diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 60b824b531..72db36035d 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -162,58 +162,23 @@ func (c *glCanvas) Refresh(obj fyne.CanvasObject) { } func (c *glCanvas) Focus(obj fyne.Focusable) { - c.RLock() - mgr := c.focusMgr - c.RUnlock() - focused := mgr.Focused() - - if focused == obj || obj == nil { - return - } - - if dis, ok := obj.(fyne.Disableable); ok && dis.Disabled() { - c.Unfocus() - return - } - - if focused != nil { - focused.FocusLost() - } - - mgr.Focus(obj) - obj.FocusGained() + c.focusManager().Focus(obj) } func (c *glCanvas) FocusNext() { - c.RLock() - mgr := c.focusMgr - c.RUnlock() - mgr.FocusNext() + c.focusManager().FocusNext() } func (c *glCanvas) FocusPrevious() { - c.RLock() - mgr := c.focusMgr - c.RUnlock() - mgr.FocusPrevious() + c.focusManager().FocusPrevious() } func (c *glCanvas) Unfocus() { - c.RLock() - mgr := c.focusMgr - c.RUnlock() - focused := mgr.Focused() - - if focused != nil { - mgr.Focus(nil) - focused.FocusLost() - } + c.focusManager().Focus(nil) } func (c *glCanvas) Focused() fyne.Focusable { - c.RLock() - defer c.RUnlock() - return c.focusMgr.Focused() + return c.focusManager().Focused() } func (c *glCanvas) Resize(size fyne.Size) { @@ -418,6 +383,12 @@ func (c *glCanvas) ensureMinSize() bool { return windowNeedsMinSizeUpdate } +func (c *glCanvas) focusManager() *app.FocusManager { + c.RLock() + defer c.RUnlock() + return c.focusMgr +} + func (c *glCanvas) isDirty() bool { c.dirtyMutex.Lock() defer c.dirtyMutex.Unlock() From 83b4f9e6eb159499000a0f0c8eacc8789508a1bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilo=20Pr=C3=BCtz?= Date: Sat, 12 Sep 2020 08:54:13 +0200 Subject: [PATCH 7/7] fix focus next/previous bug With the move of focus next/previous to FocusManager the action lost the ability to notify the Focusable. This commit restores it and adds appropriate tests. --- internal/app/focus.go | 39 ++++++++++++++------------- internal/app/focus_test.go | 54 +++++++++++++++++++++++++++++++------- 2 files changed, 66 insertions(+), 27 deletions(-) diff --git a/internal/app/focus.go b/internal/app/focus.go index 80a4d4e6b4..4cd9e19d0d 100644 --- a/internal/app/focus.go +++ b/internal/app/focus.go @@ -24,22 +24,7 @@ func NewFocusManager(c fyne.CanvasObject) *FocusManager { func (f *FocusManager) Focus(obj fyne.Focusable) { f.Lock() defer f.Unlock() - - if f.focused == obj { - return - } - - if dis, ok := obj.(fyne.Disableable); ok && dis.Disabled() { - obj = nil - } - - if f.focused != nil { - f.focused.FocusLost() - } - f.focused = obj - if obj != nil { - obj.FocusGained() - } + f.focus(obj) } // Focused returns the currently focused object or nil if none. @@ -54,7 +39,7 @@ func (f *FocusManager) Focused() fyne.Focusable { func (f *FocusManager) FocusNext() { f.Lock() defer f.Unlock() - f.focused = f.nextInChain(f.focused) + f.focus(f.nextInChain(f.focused)) } // FocusPrevious will find the item before the current that can be focused and focus it. @@ -62,7 +47,25 @@ func (f *FocusManager) FocusNext() { func (f *FocusManager) FocusPrevious() { f.Lock() defer f.Unlock() - f.focused = f.previousInChain(f.focused) + f.focus(f.previousInChain(f.focused)) +} + +func (f *FocusManager) focus(obj fyne.Focusable) { + if f.focused == obj { + return + } + + if dis, ok := obj.(fyne.Disableable); ok && dis.Disabled() { + obj = nil + } + + if f.focused != nil { + f.focused.FocusLost() + } + f.focused = obj + if obj != nil { + obj.FocusGained() + } } func (f *FocusManager) nextInChain(current fyne.Focusable) fyne.Focusable { diff --git a/internal/app/focus_test.go b/internal/app/focus_test.go index b812b837c7..21e08b32c9 100644 --- a/internal/app/focus_test.go +++ b/internal/app/focus_test.go @@ -10,13 +10,13 @@ import ( ) func TestFocusManager_Focus(t *testing.T) { - entry1 := widget.NewEntry() + entry1 := &focusable{} hidden := widget.NewCheck("test", func(bool) {}) hidden.Hide() - entry2 := widget.NewEntry() + entry2 := &focusable{} disabled := widget.NewCheck("test", func(bool) {}) disabled.Disable() - entry3 := widget.NewEntry() + entry3 := &focusable{} c := widget.NewVBox(entry1, hidden, entry2, disabled, entry3) manager := app.NewFocusManager(c) @@ -24,25 +24,31 @@ func TestFocusManager_Focus(t *testing.T) { manager.Focus(entry2) assert.Equal(t, entry2, manager.Focused()) + assert.True(t, entry2.focused) manager.Focus(entry1) assert.Equal(t, entry1, manager.Focused()) + assert.True(t, entry1.focused) + assert.False(t, entry2.focused) manager.Focus(entry3) assert.Equal(t, entry3, manager.Focused()) + assert.True(t, entry3.focused) + assert.False(t, entry1.focused) manager.Focus(nil) assert.Nil(t, manager.Focused()) + assert.False(t, entry3.focused) } func TestFocusManager_FocusNext(t *testing.T) { - entry1 := widget.NewEntry() + entry1 := &focusable{} hidden := widget.NewCheck("test", func(bool) {}) hidden.Hide() - entry2 := widget.NewEntry() + entry2 := &focusable{} disabled := widget.NewCheck("test", func(bool) {}) disabled.Disable() - entry3 := widget.NewEntry() + entry3 := &focusable{} c := widget.NewVBox(entry1, hidden, entry2, disabled, entry3) manager := app.NewFocusManager(c) @@ -50,25 +56,32 @@ func TestFocusManager_FocusNext(t *testing.T) { manager.FocusNext() assert.Equal(t, entry1, manager.Focused()) + assert.True(t, entry1.focused) manager.FocusNext() assert.Equal(t, entry2, manager.Focused()) + assert.True(t, entry2.focused) + assert.False(t, entry1.focused) manager.FocusNext() assert.Equal(t, entry3, manager.Focused()) + assert.True(t, entry3.focused) + assert.False(t, entry2.focused) manager.FocusNext() assert.Equal(t, entry1, manager.Focused()) + assert.True(t, entry1.focused) + assert.False(t, entry3.focused) } func TestFocusManager_FocusPrevious(t *testing.T) { - entry1 := widget.NewEntry() + entry1 := &focusable{} hidden := widget.NewCheck("test", func(bool) {}) hidden.Hide() - entry2 := widget.NewEntry() + entry2 := &focusable{} disabled := widget.NewCheck("test", func(bool) {}) disabled.Disable() - entry3 := widget.NewEntry() + entry3 := &focusable{} c := widget.NewVBox(entry1, hidden, entry2, disabled, entry3) manager := app.NewFocusManager(c) @@ -76,13 +89,36 @@ func TestFocusManager_FocusPrevious(t *testing.T) { manager.FocusPrevious() assert.Equal(t, entry3, manager.Focused()) + assert.True(t, entry3.focused) manager.FocusPrevious() assert.Equal(t, entry2, manager.Focused()) + assert.True(t, entry2.focused) + assert.False(t, entry3.focused) manager.FocusPrevious() assert.Equal(t, entry1, manager.Focused()) + assert.True(t, entry1.focused) + assert.False(t, entry2.focused) manager.FocusPrevious() assert.Equal(t, entry3, manager.Focused()) + assert.True(t, entry3.focused) + assert.False(t, entry1.focused) +} + +type focusable struct { + widget.Entry + focused bool +} + +func (f *focusable) FocusGained() { + if f.Disabled() { + return + } + f.focused = true +} + +func (f *focusable) FocusLost() { + f.focused = false }