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
Add window states API #2473
base: main
Are you sure you want to change the base?
Add window states API #2473
Changes from all commits
d82c615
96ef3ed
4657567
00491c8
41c1dd7
60d5e43
f497a1d
d34cb2f
4d790b2
18b0cbc
618d647
b7eb117
cadc4cc
bf3511a
d61a644
5b58045
c9a1307
ab9b778
0a90a12
73721b3
c809698
06b7f2a
11fea69
938504c
be87113
9aefd30
13d2ff3
176e879
1ba73b8
d4bfdaa
d993b80
38bf5c3
b710eaf
b461295
0c7900b
97a6c66
ba28194
c7320ce
aed5251
a76bf9b
f0d5d80
e416f20
1fa8cde
6350b5f
d091955
5c2a53b
564c5ed
e36e596
174d66b
17564a8
51cea93
feb78a8
4632738
8286b86
16d5758
5b59b14
85dd202
552c0fc
56edec1
971b8da
934b6b1
1e6bd76
426e8b9
ccef113
4d83d19
067e988
3c84a80
a276f22
78cdd3b
b9e87c5
06d5a9e
b0c37fe
09ee428
1bd1f39
2b6d347
8bba100
6b3e883
74685d1
25c420f
83dcab4
ab0ed45
4fa4af6
71f4314
a308ea7
42642ee
6b513c7
ccc8f11
56f32a7
5da8231
4c6070d
cfa255a
87383f3
c3f32c7
4b2fdbb
b592ee2
a5891e8
e8ad898
8684038
1b53c16
3cebc76
d2510e0
4485cda
d0dcb35
f429307
6aedeea
d7228ba
600e4c9
3689314
1a6b4ad
2885d1c
cd9e00b
57fd1e3
7835fe8
1c0a969
cf23ce9
e6f4dcc
30e5968
c623254
f2bd750
11444e3
0771349
c9b7b90
3441cb3
62dcd7d
4f36f28
ea8058a
11c3ec3
0bd68c4
edef651
3c8b0f1
e915d72
fb6e657
b810d65
1e59da1
9dba56b
56cc124
2d29072
c2e88e5
279dfda
73bf7aa
af6a292
5a0352f
ef68572
ca48ba2
cd8eba2
e5eff2b
26eb01c
3a6aaf7
78b5ddf
e0f5bf0
54cfaab
8b8685e
d599730
91b70da
b38ff03
2cf1839
aa120e9
3661110
c928ac1
f9c18f5
ace13ce
ef7b3dc
bc19c7f
c2235f7
b0ead86
7bbf330
0f7fe4f
7308e11
8f858be
9f95554
295cd59
5bd410e
00a742e
4ef9c50
a1613be
138d0e3
450efad
51d3590
3de8eef
77e2b7e
7db80d2
aaee1f9
903860b
a6df392
f9f1375
ad427ab
56de33d
59012cd
5a66725
bb22a9f
f6a851d
35bbb8e
8bddf0e
03dd538
2af1509
8b56d66
7cd1489
598021f
596acc4
d3f90ba
945caf5
116eec3
71aec38
2559fa0
d822ff0
14b64b7
d8c8422
bb513d3
eb2d9a5
abfa707
8f7c954
379e002
e3f31ff
383e027
c986e3d
5f365c6
2961507
9ca14ed
b6edc2a
0f74ad9
589e3c7
621c723
655a146
4f2f2a5
c29ab39
0d67322
07e82f9
8be4d8d
1692a0d
f8ee69d
b82b6d0
247847e
1b624ad
5560e3d
a8fed19
5f0c1a2
136cdcf
8fcdbd6
ef584b1
7f28348
6e0c091
f535cd6
c175b6a
bc1f902
e8864d5
0f0103c
4162e4b
eb51730
b25b902
8df6c4b
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.
It seems like this should be an optimisation at the core level - it's something that will be common to every implementation.
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 can't be done at the core level because we can't reliably determine the current state. Backends like Cocoa and GTK use non-blocking state APIs, meaning the state might be in in-between transition when checked. As a result, we could end up comparing against an outdated state.
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.
Sure - but that means there are 2 possible interpretations of "window state" - the "current state", and the "state we're transitioning to". On blocking APIs, the two are always the same; on non-blocking, they might be different. This is easy enough to accomodate with a boolean argument to
get_window_state()
that allows the user to explicitly request "actual" state or "in progress" state.For the purpose of the public API, I'd argue you always want the "state we're transitioning to" - as that is the answer that will be consistent across platforms, and will ensure that:
will always return the expected result. However, it would warrant a note on the
state
accessor to highlight that the value returned might not reflect the actual state immediately after making a request.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 have modified
get_window_state()
and moved same state checking to the core. I have also moved same state transition tests to the core.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.
In which case - is the check right here still 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.
No, not really, since the Android API is blocking in nature and the internal API
set_window_state()
is not called internally anywhere on the implementation side.It is only required on the non-blocking API backends like cocoa, as the internal API
_apply_state()
is called internally on the implementation side at different places.So, I have removed it from the Android backend.
EDIT: On further testing, this check acts as a termination condition for some cases when
set_window_state()
is called internally. So, I have kept it.