Skip to content

Commit

Permalink
fix fyne-io#1354: switching overlays no longer removes focus from pre…
Browse files Browse the repository at this point in the history
…vious focus manager

Thus the focused element in the content stays highlighted while a pop-up
menu is opened. Also the focused element stays highlighted if the menu
is opened.
However, the _one_ focus for keyboard events is moved to the overlay on top.
  • Loading branch information
toaster committed Oct 6, 2020
1 parent 39777d2 commit fbb11d6
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 29 deletions.
10 changes: 1 addition & 9 deletions internal/driver/glfw/canvas.go
Original file line number Diff line number Diff line change
Expand Up @@ -388,18 +388,10 @@ func (c *glCanvas) objectTrees() []fyne.CanvasObject {
return trees
}

func (c *glCanvas) overlayChanged(focusMgr, prevFocusMgr *app.FocusManager) {
func (c *glCanvas) overlayChanged() {
c.Lock()
defer c.Unlock()
c.dirty = true
if prevFocusMgr == nil {
prevFocusMgr = c.focusMgr
}
if focusMgr == nil {
focusMgr = c.focusMgr
}
prevFocusMgr.FocusLost()
focusMgr.FocusGained()
}

func (c *glCanvas) paint(size fyne.Size) {
Expand Down
20 changes: 10 additions & 10 deletions internal/driver/glfw/canvas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -188,26 +188,26 @@ func TestGlCanvas_FocusHandlingWhenAddingAndRemovingOverlays(t *testing.T) {
assert.True(t, ce2.focused)

c.Overlays().Add(overlay1)
ctxt := "adding overlay removes focus from content"
ctxt := "adding overlay changes focus handler but does not remove focus from content"
assert.Nil(t, c.Focused(), ctxt)
assert.False(t, ce2.focused, ctxt)
assert.True(t, ce2.focused, ctxt)

c.FocusNext()
ctxt = "changing focus affects overlay instead of content"
assert.Equal(t, o1e1, c.Focused(), ctxt)
assert.False(t, ce1.focused, ctxt)
assert.False(t, ce2.focused, ctxt)
assert.True(t, ce2.focused, ctxt)
assert.True(t, o1e1.focused, ctxt)

c.Overlays().Add(overlay2)
ctxt = "adding overlay removes focus from previous overlay"
ctxt = "adding overlay changes focus handler but does not remove focus from previous overlay"
assert.Nil(t, c.Focused(), ctxt)
assert.False(t, o1e1.focused, ctxt)
assert.True(t, o1e1.focused, ctxt)

c.FocusPrevious()
ctxt = "changing focus affects top overlay only"
assert.Equal(t, o2e2, c.Focused(), ctxt)
assert.False(t, o1e1.focused, ctxt)
assert.True(t, o1e1.focused, ctxt)
assert.False(t, o1e2.focused, ctxt)
assert.True(t, o2e2.focused, ctxt)

Expand All @@ -217,9 +217,9 @@ func TestGlCanvas_FocusHandlingWhenAddingAndRemovingOverlays(t *testing.T) {
assert.True(t, o2e1.focused)

c.Overlays().Remove(overlay2)
ctxt = "removing overlay removes focus from removed overlay and restores focus on new top overlay"
ctxt = "removing overlay restores focus handler from previous overlay but does not remove focus from removed overlay"
assert.Equal(t, o1e1, c.Focused(), ctxt)
assert.False(t, o2e1.focused, ctxt)
assert.True(t, o2e1.focused, ctxt)
assert.False(t, o2e2.focused, ctxt)
assert.True(t, o1e1.focused, ctxt)

Expand All @@ -229,10 +229,10 @@ func TestGlCanvas_FocusHandlingWhenAddingAndRemovingOverlays(t *testing.T) {
assert.True(t, o1e2.focused)

c.Overlays().Remove(overlay1)
ctxt = "removing last overlay removes focus from removed overlay and restores focus on content"
ctxt = "removing last overlay restores focus handler from content but does not remove focus from removed overlay"
assert.Equal(t, ce2, c.Focused(), ctxt)
assert.False(t, o1e1.focused, ctxt)
assert.False(t, o1e2.focused, ctxt)
assert.True(t, o1e2.focused, ctxt)
assert.True(t, ce2.focused, ctxt)
}

Expand Down
17 changes: 7 additions & 10 deletions internal/overlay_stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (

// OverlayStack implements fyne.OverlayStack
type OverlayStack struct {
OnChange func(focusMgr, prevFocusMgr *app.FocusManager)
OnChange func()
focusManagers []*app.FocusManager
overlays []fyne.CanvasObject
propertyLock sync.RWMutex
Expand All @@ -24,12 +24,14 @@ func (s *OverlayStack) Add(overlay fyne.CanvasObject) {
s.propertyLock.Lock()
defer s.propertyLock.Unlock()

defer s.notifyAboutChange(s.topFocusManager())
if overlay == nil {
return
}
s.overlays = append(s.overlays, overlay)
s.focusManagers = append(s.focusManagers, app.NewFocusManager(overlay))
if s.OnChange != nil {
s.OnChange()
}
}

// List returns all overlays on the stack from bottom to top.
Expand All @@ -49,14 +51,16 @@ func (s *OverlayStack) Remove(overlay fyne.CanvasObject) {
s.propertyLock.Lock()
defer s.propertyLock.Unlock()

defer s.notifyAboutChange(s.topFocusManager())
for i, o := range s.overlays {
if o == overlay {
s.overlays = s.overlays[:i]
s.focusManagers = s.focusManagers[:i]
break
}
}
if s.OnChange != nil {
s.OnChange()
}
}

// Top returns the top-most overlay of the stack.
Expand All @@ -79,13 +83,6 @@ func (s *OverlayStack) TopFocusManager() *app.FocusManager {
return s.topFocusManager()
}

func (s *OverlayStack) notifyAboutChange(pfm *app.FocusManager) {
if s.OnChange == nil {
return
}
s.OnChange(s.topFocusManager(), pfm)
}

func (s *OverlayStack) topFocusManager() *app.FocusManager {
var fm *app.FocusManager
if len(s.focusManagers) > 0 {
Expand Down

0 comments on commit fbb11d6

Please sign in to comment.