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
Added
on_gain_focus
,on_lose_focus
,on_show
&on_hide
handlers ontoga.Window
#2096base: main
Are you sure you want to change the base?
Added
on_gain_focus
,on_lose_focus
,on_show
&on_hide
handlers ontoga.Window
#2096Changes from 75 commits
6e2eda0
4bb2a42
3a202e4
9cecb4a
60a3cac
87ea0a4
6f38ae2
6dce43e
3133dfb
3b03ec4
d746ba8
096e6bf
943612d
7b864fe
88c56b9
156bc9e
aa4714e
6895d8d
27bf2a7
9fabc3e
7b23209
0427a8f
b9e0412
7f7468d
429568b
3440221
fc8a29c
6fd5e3c
44bade9
8604faa
84234e8
eaeaf5d
ea28017
894a89c
cb2e541
f7a014b
0a1d093
c441e77
059ea14
81dd316
d958394
d94a69a
8c364a1
c0405c3
927b3d4
869da6a
2423548
9fd0e0e
77b8822
89f095d
69f90d5
d5e8e3f
52e75b8
dc8b52d
004a98a
2fa2bfa
246eeb0
673e2cd
5cf9bbe
bfe62fa
e65c528
5273e54
69f3f25
30cb114
40d0320
f25ee59
11c6323
4063113
c3a55d8
cc9a023
43b895f
b072c6b
8d0477d
a017847
cb75101
9b9b569
db8e5e4
e3fc53b
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.
Is iterating over all windows appropriate? Won't it just be
current_window
that gains focus? Admittedly, this is mostly a moot point as long as Toga doesn't support multiple windows on Android, but in the event that we ever do support multiple windows (e.g., supporting an external display), it's worth getting the logic right here.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 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 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 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 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 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.
I had written these core tests when there was no API for window states. Currently, I have added new tests that test the behavior of triggering the events with changes in window state. So, should I keep these tests?
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.
They're not testing anything that isn't covered by the "real" focus/show tests, so I'd say they're not needed.
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.
These calls assertions will be potentially misleading, as the mocks haven't been reset between each use. The test also isn't asserting the handlers that aren't called.
Given that this will be a repeated pattern, it might be worth wrapping this sequence of calls in a utility assert methods.
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.
AFAIR,
get_current_window()
can return None; but if the window exists, it will always have an interface attribute. On that basis, this getattr() is protecting against the wrong missing attribute.