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

window. add close button interceptor to react on the window closing before anything is done #1180

Merged
merged 9 commits into from Aug 14, 2020
Merged

window. add close button interceptor to react on the window closing before anything is done #1180

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
75e1ce9
window. add willClose handler to react on the window closing before a…
Jul 10, 2020
2c0e228
window. implement willClose for mobile
Jul 12, 2020
b492c0e
window. willClose to replace internal closing procedure
Jul 12, 2020
1f15580
window. add DoClose method closing window without pre-checks
Jul 13, 2020
b9e10a4
window. rename to SetCloseIntercept and minimize other changes
Jul 15, 2020
83f2c83
window. move interception out of Close so Close is simply closing the…
Jul 15, 2020
4e37d25
window. push close interceptor to the queueEvent instead of direct call.
Aug 11, 2020
69be504
window. test close interception functionality
Aug 11, 2020
6711148
window. close interception tests to check onClosed calls
Aug 11, 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
37 changes: 25 additions & 12 deletions internal/driver/glfw/window.go
View file
Open in desktop
Expand Up @@ -74,6 +74,7 @@ type window struct {
mouseLastClick fyne.CanvasObject
mousePressed fyne.CanvasObject
onClosed func()
onCloseIntercepted func()

xpos, ypos int
width, height int
Expand Down Expand Up @@ -292,6 +293,10 @@ func (w *window) SetOnClosed(closed func()) {
w.onClosed = closed
}

func (w *window) SetCloseIntercept(callback func()) {
w.onCloseIntercepted = callback
}

func (w *window) getMonitorForWindow() *glfw.Monitor {
xOff := w.xpos + (w.width / 2)
yOff := w.ypos + (w.height / 2)
Expand Down Expand Up @@ -404,7 +409,20 @@ func (w *window) Close() {
if w.viewport == nil {
return
}
w.closed(w.viewport)

w.viewport.SetShouldClose(true)

w.canvas.walkTrees(nil, func(node *renderCacheNode) {
switch co := node.obj.(type) {
case fyne.Widget:
cache.DestroyRenderer(co)
}
})

// trigger callbacks
if w.onClosed != nil {
w.queueEvent(w.onClosed)
}
}

func (w *window) ShowAndRun() {
Expand Down Expand Up @@ -446,19 +464,14 @@ func (w *window) Canvas() fyne.Canvas {
}

func (w *window) closed(viewport *glfw.Window) {
viewport.SetShouldClose(true)
viewport.SetShouldClose(false)

w.canvas.walkTrees(nil, func(node *renderCacheNode) {
switch co := node.obj.(type) {
case fyne.Widget:
cache.DestroyRenderer(co)
}
})

// trigger callbacks
if w.onClosed != nil {
w.queueEvent(w.onClosed)
if w.onCloseIntercepted != nil {
andydotxyz marked this conversation as resolved.
Show resolved Hide resolved
w.onCloseIntercepted()
Copy link
Member

Choose a reason for hiding this comment

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

Earlier you mentioned that this could block - should it be passed to w.queueEvent instead?

Copy link
Author

Choose a reason for hiding this comment

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

i've tried that, and it's not helping unfortunately
i can do this to follow the pattern

Copy link
Member

Choose a reason for hiding this comment

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

Can you confirm what the issue is please?
Looking at the example above I can't help but feel the channel should not be needed - just calling Window.Close() in the callback should work.

Copy link
Author

@ghost ghost Aug 11, 2020 •
edited by ghost

Choose a reason for hiding this comment

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

the issue is not reacting ShowConfirm
Yes, it's solved with removing the channel with and without using queueEvent. Changed the example accordingly

return
}

w.Close()
}

// destroy this window and, if it's the last window quit the app
Expand Down
24 changes: 19 additions & 5 deletions internal/driver/gomobile/window.go
View file
Open in desktop
Expand Up @@ -9,10 +9,11 @@ import (
)

type window struct {
title string
visible bool
onClosed func()
isChild bool
title string
visible bool
onClosed func()
onCloseIntercepted func()
isChild bool

clipboard fyne.Clipboard
canvas *mobileCanvas
Expand Down Expand Up @@ -92,6 +93,10 @@ func (w *window) SetOnClosed(callback func()) {
w.onClosed = callback
}

func (w *window) SetCloseIntercept(callback func()) {
w.onCloseIntercepted = callback
}

func (w *window) Show() {
menu := fyne.CurrentApp().Driver().(*mobileDriver).findMenu(w)
menuButton := widget.NewButtonWithIcon("", theme.MenuIcon(), func() {
Expand All @@ -103,7 +108,7 @@ func (w *window) Show() {

if w.isChild {
exit := widget.NewButtonWithIcon("", theme.CancelIcon(), func() {
w.Close()
w.tryClose()
})
title := widget.NewLabel(w.title)
title.Alignment = fyne.TextAlignCenter
Expand All @@ -129,6 +134,15 @@ func (w *window) Hide() {
}
}

func (w *window) tryClose() {
if w.onCloseIntercepted != nil {
andydotxyz marked this conversation as resolved.
Show resolved Hide resolved
w.onCloseIntercepted()
return
}

w.Close()
}

func (w *window) Close() {
d := fyne.CurrentApp().Driver().(*mobileDriver)
pos := -1
Expand Down
15 changes: 10 additions & 5 deletions test/testwindow.go
View file
Open in desktop
Expand Up @@ -5,11 +5,12 @@ import (
)

type testWindow struct {
title string
fullScreen bool
fixedSize bool
focused bool
onClosed func()
title string
fullScreen bool
fixedSize bool
focused bool
onClosed func()
onCloseIntercepted func()

canvas *testCanvas
clipboard fyne.Clipboard
Expand Down Expand Up @@ -112,6 +113,10 @@ func (w *testWindow) SetOnClosed(closed func()) {
w.onClosed = closed
}

func (w *testWindow) SetCloseIntercept(callback func()) {
w.onCloseIntercepted = callback
}

func (w *testWindow) SetPadded(padded bool) {
w.canvas.SetPadded(padded)
}
Expand Down
3 changes: 3 additions & 0 deletions window.go
View file
Open in desktop
Expand Up @@ -64,6 +64,9 @@ type Window interface {
// SetOnClosed sets a function that runs when the window is closed.
SetOnClosed(func())

// SetCloseIntercept sets a function that runs instead of closing if defined
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a "." to this, and add another line instructing people to call Close() to confirm window closure?

SetCloseIntercept(func())

// Show the window on screen.
Show()
// Hide the window from the user.
Expand Down