Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
Refactor window closing and app exit handling #980
Refactor window closing and app exit handling #980
Changes from 38 commits
ea28928
c5c8232
7f88423
452aa49
7b2bdac
c77bfbe
c015b2c
64806ed
1987af2
70540fb
c214df8
8dcf638
9a175c7
189945f
36b2252
2842bff
7dfabf0
7ced499
246ef5c
cd5996a
42ab90c
f3643a8
33d4921
cf6d32b
5d64c23
6d0c7d3
fe53938
9b945cf
beea045
f8ac0b1
dc12978
d937748
5fa9a35
b36ce6b
01e6cec
17ff511
62313e5
3099f96
fa3891b
105c388
a8dbfc9
e2a2593
ebf3c88
2b871ec
c791b9b
6b21942
4fc1e1d
a61f936
04ed041
cabef46
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A DocumentApp won't have a MainWindow; there's a window for each Document, so this isn't a concern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This effectively duplicates the logic of
Window.toga_on_close
; ideally, we'd re-use the base class logic, and add the extra handling for allowing/starting the exit.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type annotation here is significant. Without it, the return type is assumed to be "object", and every value returned will be true.
windowShouldClose:
returns Boolean; so if we want True/False to be interpreted correctly, the method needs to be annotated-> bool
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's an edge case here. This will unconditionally remove the window from the app; but if you return
False
in youron_close
handler, the window won't be closed. That means we end up with a window that isn't being tracked.