From f8a2ae6dc90e6fdfb881a6e524f7681e5e8354d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilo=20Pr=C3=BCtz?= Date: Fri, 23 Oct 2020 22:09:29 +0200 Subject: [PATCH 1/2] wip fix 814: foxus handling for non-native menus --- internal/driver/glfw/canvas.go | 47 ++++++++++-------- internal/driver/glfw/canvas_other_test.go | 60 +++++++++++++++++++++++ internal/driver/glfw/driver_test.go | 2 +- internal/driver/glfw/menu.go | 2 +- internal/driver/glfw/menu_bar.go | 6 +++ internal/driver/glfw/window.go | 12 ++--- 6 files changed, 99 insertions(+), 30 deletions(-) create mode 100644 internal/driver/glfw/canvas_other_test.go diff --git a/internal/driver/glfw/canvas.go b/internal/driver/glfw/canvas.go index 12369e93e5..6c670926a0 100644 --- a/internal/driver/glfw/canvas.go +++ b/internal/driver/glfw/canvas.go @@ -22,12 +22,13 @@ var _ fyne.Canvas = (*glCanvas)(nil) type glCanvas struct { sync.RWMutex - content fyne.CanvasObject - menu fyne.CanvasObject - overlays *overlayStack - padded bool - size fyne.Size - focusMgr *app.FocusManager + content fyne.CanvasObject + contentFocusMgr *app.FocusManager + menu fyne.CanvasObject + menuFocusMgr *app.FocusManager + overlays *overlayStack + padded bool + size fyne.Size onTypedRune func(rune) onTypedKey func(*fyne.KeyEvent) @@ -253,6 +254,8 @@ func (c *glCanvas) Unfocus() { } func (c *glCanvas) buildMenu(w *window, m *fyne.MainMenu) { + c.Lock() + defer c.Unlock() c.setMenuOverlay(nil) if m == nil { return @@ -264,6 +267,10 @@ func (c *glCanvas) buildMenu(w *window, m *fyne.MainMenu) { } } +func (c *glCanvas) RemoveShortcut(shortcut fyne.Shortcut) { + c.shortcut.RemoveShortcut(shortcut) +} + // 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())) @@ -274,10 +281,6 @@ func (c *glCanvas) canvasSize(contentSize fyne.Size) fyne.Size { return canvasSize } -func (c *glCanvas) RemoveShortcut(shortcut fyne.Shortcut) { - c.shortcut.RemoveShortcut(shortcut) -} - func (c *glCanvas) contentPos() fyne.Position { contentPos := fyne.NewPos(0, c.menuHeight()) if c.Padded() { @@ -358,7 +361,10 @@ func (c *glCanvas) focusManager() *app.FocusManager { if focusMgr := c.overlays.TopFocusManager(); focusMgr != nil { return focusMgr } - return c.focusMgr + if c.isMenuActive() { + return c.menuFocusMgr + } + return c.contentFocusMgr } func (c *glCanvas) isDirty() bool { @@ -368,20 +374,20 @@ func (c *glCanvas) isDirty() bool { return c.dirty } +func (c *glCanvas) isMenuActive() bool { + return c.menu != nil && c.menu.(*MenuBar).IsActive() +} + func (c *glCanvas) menuHeight() int { - switch c.menuOverlay() { + switch c.menu { case nil: // no menu or native menu -> does not consume space on the canvas return 0 default: - return c.menuOverlay().MinSize().Height + return c.menu.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) @@ -428,7 +434,7 @@ func (c *glCanvas) paint(size fyne.Size) { func (c *glCanvas) setContent(content fyne.CanvasObject) { c.content = content c.contentTree = &renderCacheTree{root: &renderCacheNode{obj: c.content}} - c.focusMgr = app.NewFocusManager(c.content) + c.contentFocusMgr = app.NewFocusManager(c.content) } func (c *glCanvas) setDirty(dirty bool) { @@ -438,11 +444,10 @@ func (c *glCanvas) setDirty(dirty bool) { c.dirty = dirty } -func (c *glCanvas) setMenuOverlay(b fyne.CanvasObject) { - c.Lock() +func (c *glCanvas) setMenuOverlay(b *MenuBar) { c.menu = b c.menuTree = &renderCacheTree{root: &renderCacheNode{obj: c.menu}} - c.Unlock() + c.menuFocusMgr = app.NewFocusManager(c.menu) } func (c *glCanvas) setupThemeListener() { diff --git a/internal/driver/glfw/canvas_other_test.go b/internal/driver/glfw/canvas_other_test.go new file mode 100644 index 0000000000..11c06a32f6 --- /dev/null +++ b/internal/driver/glfw/canvas_other_test.go @@ -0,0 +1,60 @@ +// +build !mobile +// +build !darwin no_native_menus + +package glfw + +import ( + "testing" + + "fyne.io/fyne" + "fyne.io/fyne/container" + "github.com/stretchr/testify/assert" +) + +func TestGlCanvas_FocusHandlingWhenActivatingOrDeactivatingTheMenu(t *testing.T) { + w := createWindow("Test") + w.SetPadded(false) + w.SetMainMenu( + fyne.NewMainMenu( + fyne.NewMenu("test", fyne.NewMenuItem("item", func() {})), + ), + ) + c := w.Canvas().(*glCanvas) + + ce1 := &focusable{id: "ce1"} + ce2 := &focusable{id: "ce2"} + content := container.NewVBox(ce1, ce2) + w.SetContent(content) + + assert.Nil(t, c.Focused()) + m := c.menu.(*MenuBar) + assert.False(t, m.IsActive()) + + c.FocusPrevious() + assert.Equal(t, ce2, c.Focused()) + assert.True(t, ce2.focused) + + m.Items[0].(*menuBarItem).Tapped(&fyne.PointEvent{}) + assert.True(t, m.IsActive()) + ctxt := "activating the menu changes focus handler but does not remove focus from content" + assert.True(t, ce2.focused, ctxt) + assert.Nil(t, c.Focused(), ctxt) + + c.FocusNext() + // TODO: changes menu focus as soon as menu has focus support + ctxt = "changing focus with active menu does not affect content focus" + assert.True(t, ce2.focused, ctxt) + assert.Nil(t, c.Focused(), ctxt) + + m.Items[0].(*menuBarItem).Tapped(&fyne.PointEvent{}) + assert.False(t, m.IsActive()) + // TODO: does not remove focus from focused menu item as soon as menu has focus support + ctxt = "deactivating the menu restores focus handler from content" + assert.True(t, ce2.focused, ctxt) + assert.Equal(t, ce2, c.Focused()) + + c.FocusPrevious() + assert.Equal(t, ce1, c.Focused(), ctxt) + assert.True(t, ce1.focused, ctxt) + assert.False(t, ce2.focused, ctxt) +} diff --git a/internal/driver/glfw/driver_test.go b/internal/driver/glfw/driver_test.go index d951f874df..1f83a2afd4 100644 --- a/internal/driver/glfw/driver_test.go +++ b/internal/driver/glfw/driver_test.go @@ -56,7 +56,7 @@ func Test_gLDriver_AbsolutePositionForObject(t *testing.T) { // 0 is the shadow // 1 is the menu bar’s background // 2 is the container holding the items - mbarCont := cache.Renderer(movl.(fyne.Widget)).Objects()[2].(*fyne.Container) + mbarCont := cache.Renderer(movl).Objects()[2].(*fyne.Container) m2 := mbarCont.Objects[1] tests := map[string]struct { diff --git a/internal/driver/glfw/menu.go b/internal/driver/glfw/menu.go index b7604ddc4d..649acf4f8f 100644 --- a/internal/driver/glfw/menu.go +++ b/internal/driver/glfw/menu.go @@ -4,7 +4,7 @@ import ( "fyne.io/fyne" ) -func buildMenuOverlay(menus *fyne.MainMenu, c fyne.Canvas) fyne.CanvasObject { +func buildMenuOverlay(menus *fyne.MainMenu, c fyne.Canvas) *MenuBar { if len(menus.Items) == 0 { fyne.LogError("Main menu must have at least one child menu", nil) return nil diff --git a/internal/driver/glfw/menu_bar.go b/internal/driver/glfw/menu_bar.go index 6e7a78c1e6..161a04b4bb 100644 --- a/internal/driver/glfw/menu_bar.go +++ b/internal/driver/glfw/menu_bar.go @@ -51,6 +51,12 @@ func (b *MenuBar) CreateRenderer() fyne.WidgetRenderer { } } +// IsActive returns whether the menu bar is active or not. +// An active menu bar shows the current selected menu and should have the focus. +func (b *MenuBar) IsActive() bool { + return b.active +} + // Hide hides the menu bar. // // Implements: fyne.Widget diff --git a/internal/driver/glfw/window.go b/internal/driver/glfw/window.go index 923c05b7e4..37fb8d56bc 100644 --- a/internal/driver/glfw/window.go +++ b/internal/driver/glfw/window.go @@ -643,7 +643,7 @@ func (w *window) mouseOut() { } func (w *window) mouseClicked(_ *glfw.Window, btn glfw.MouseButton, action glfw.Action, mods glfw.ModifierKey) { - co, pos, layer := w.findObjectAtPositionMatching(w.canvas, w.mousePos, func(object fyne.CanvasObject) bool { + co, pos, _ := w.findObjectAtPositionMatching(w.canvas, w.mousePos, func(object fyne.CanvasObject) bool { switch object.(type) { case fyne.Tappable, fyne.SecondaryTappable, fyne.DoubleTappable, fyne.Focusable, fyne.Draggable, desktop.Mouseable, desktop.Hoverable: return true @@ -675,12 +675,10 @@ func (w *window) mouseClicked(_ *glfw.Window, btn glfw.MouseButton, action glfw. } } - if layer != 1 { // 0 - overlay, 1 - menu, 2 - content - if wid, ok := co.(fyne.Focusable); ok { - w.canvas.Focus(wid) - } else { - w.canvas.Unfocus() - } + if wid, ok := co.(fyne.Focusable); ok { + w.canvas.Focus(wid) + } else { + w.canvas.Unfocus() } if action == glfw.Press { From 93ff2d43a5acc67da66140d25039cbc09adb993c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tilo=20Pr=C3=BCtz?= Date: Sat, 24 Oct 2020 09:18:54 +0200 Subject: [PATCH 2/2] canvas test do not run on CI (yet) --- internal/driver/glfw/canvas_other_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/driver/glfw/canvas_other_test.go b/internal/driver/glfw/canvas_other_test.go index 11c06a32f6..f7e2ab4efe 100644 --- a/internal/driver/glfw/canvas_other_test.go +++ b/internal/driver/glfw/canvas_other_test.go @@ -1,3 +1,4 @@ +// +build !ci // +build !mobile // +build !darwin no_native_menus @@ -8,6 +9,7 @@ import ( "fyne.io/fyne" "fyne.io/fyne/container" + "github.com/stretchr/testify/assert" )