-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add some missing Window getters #4348
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
base: master
Are you sure you want to change the base?
Conversation
Implemented properly when it was easy for me, but a lot of places are stubbed out for now.
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.
Just documentation nits.
Only checked the core and Web implementation.
| /// ## Platform-specific | ||
| /// | ||
| /// - **iOS / Android / Orbital:** Unsupported. | ||
| /// - **Web:** Unimplemented, returns `None`. |
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.
| /// - **Web:** Unimplemented, returns `None`. | |
| /// - **Web:** Unimplemented. |
I propose if it returns Option, we don't have to specify that it returns None.
| /// | ||
| /// - **iOS / Android:** Unsupported. | ||
| fn set_title(&self, title: &str); | ||
| /// - **iOS / Android / Web:** Unsupported. |
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.
| /// - **iOS / Android / Web:** Unsupported. | |
| /// - **iOS / Android / Web:** Unsupported. Always returns `false`. |
I propose if it returns a bool, we should specify what it returns in case its unsupported or unimplemented.
| /// - **iOS / Android:** Unsupported. | ||
| fn set_title(&self, title: &str); | ||
| /// - **iOS / Android / Web:** Unsupported. | ||
| /// - **X11:** Unimplemented, returns `false`. |
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.
| /// - **X11:** Unimplemented, returns `false`. | |
| /// - **X11:** Unimplemented. Returns `false`. |
I propose we use two sentences. Was told before by people who know better that in English you don't use commas a whole lot, but what do I know.
| /// | ||
| /// ## Note | ||
| /// | ||
| /// - You shouldn't stop rendering for minimized windows, however you could lower the fps. |
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.
| /// - You shouldn't stop rendering for minimized windows, however you could lower the fps. | |
| /// - To support window previews you should continue rendering minimized windows. However, you could lower the frame rate. |
Add the following methods to
Window:min_surface_size.max_surface_size.is_transparent.is_blurred.window_level.window_icon.content_protected.cursor.Some of these are not implemented on all platforms yet, I have marked that in their documentation. I it to be a general theme of
Window, you should be able to query all the properties that you can set.Related to #1295 and #3690.
changelogmodule if knowledge of this change could be valuable to users