Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bugfix #948: support focus traversal in dialogs #1318

Merged
merged 8 commits into from Sep 22, 2020
Merged

Bugfix #948: support focus traversal in dialogs #1318

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
4ff0519
name glCanvas test methods consistent (and sort them)
toaster Sep 14, 2020
a082427
manage FocusManagers per overlay
toaster Sep 14, 2020
70255c3
fix #948: delegate focus handling for overlays to correct manager
toaster Sep 14, 2020
2ec5a33
add locking to glCanvas/OverlayStack
toaster Sep 18, 2020
fb92aec
inject current overlay focus manager into glCanvas.overlayChanged, too
toaster Sep 18, 2020
b69f249
fix deprecated overlay methods regarding locking
toaster Sep 18, 2020
0b06e76
add clarifying comment on `FocusGained`/`FocusLost` regarding locking
toaster Sep 22, 2020
8a1fa52
Merge branch 'develop' into bugfix/948
toaster Sep 22, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
44 changes: 38 additions & 6 deletions internal/driver/glfw/canvas.go
View file
Open in desktop
Expand Up @@ -352,6 +352,9 @@ func (c *glCanvas) ensureMinSize() bool {
func (c *glCanvas) focusManager() *app.FocusManager {
c.RLock()
defer c.RUnlock()
if focusMgr := c.overlays.TopFocusManager(); focusMgr != nil {
return focusMgr
}
return c.focusMgr
}

Expand Down Expand Up @@ -386,6 +389,21 @@ func (c *glCanvas) objectTrees() []fyne.CanvasObject {
return trees
}

func (c *glCanvas) overlayChanged(prevFocusMgr *app.FocusManager) {
c.setDirty(true)
c.RLock()
if prevFocusMgr == nil {
prevFocusMgr = c.focusMgr
}
focusMgr := c.overlays.TopFocusManager()
if focusMgr == nil {
focusMgr = c.focusMgr
}
c.RUnlock()
prevFocusMgr.FocusLost()
focusMgr.FocusGained()
Comment on lines +401 to +402
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible for prevFocusMgr == focusMgr?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! It might be possible with concurrent changes to the overlay stack.
I’ll add missing locking to the stack and also change the callback to be safe by design.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of locking, what if you just return early if they are equal?

}

func (c *glCanvas) paint(size fyne.Size) {
if c.Content() == nil {
return
Expand Down Expand Up @@ -520,23 +538,37 @@ func (c *glCanvas) walkTrees(
type overlayStack struct {
internal.OverlayStack

onChange func()
renderCaches []*renderCacheTree
focusManagers []*app.FocusManager
onChange func(prevFocusMgr *app.FocusManager)
renderCaches []*renderCacheTree
}

func (o *overlayStack) Add(overlay fyne.CanvasObject) {
if overlay == nil {
return
}
pfm := o.TopFocusManager()
o.OverlayStack.Add(overlay)
o.renderCaches = append(o.renderCaches, &renderCacheTree{root: &renderCacheNode{obj: overlay}})
o.onChange()
o.focusManagers = append(o.focusManagers, app.NewFocusManager(overlay))
o.onChange(pfm)
}

func (o *overlayStack) Remove(overlay fyne.CanvasObject) {
top := o.TopFocusManager()
o.OverlayStack.Remove(overlay)
o.renderCaches = o.renderCaches[:len(o.List())]
o.onChange()
overlayCount := len(o.List())
o.renderCaches = o.renderCaches[:overlayCount]
o.focusManagers = o.focusManagers[:overlayCount]
o.onChange(top)
}

func (o *overlayStack) TopFocusManager() *app.FocusManager {
var pfm *app.FocusManager
if len(o.focusManagers) > 0 {
pfm = o.focusManagers[len(o.focusManagers)-1]
}
return pfm
}

type renderCacheNode struct {
Expand Down Expand Up @@ -564,7 +596,7 @@ func newCanvas() *glCanvas {
c.setContent(&canvas.Rectangle{FillColor: theme.BackgroundColor()})
c.padded = true

c.overlays = &overlayStack{onChange: func() { c.setDirty(true) }}
c.overlays = &overlayStack{onChange: c.overlayChanged}

c.refreshQueue = make(chan fyne.CanvasObject, 4096)
c.dirtyMutex = &sync.Mutex{}
Expand Down