-
-
Notifications
You must be signed in to change notification settings - Fork 269
feat(mobile): add multi window support #1154
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: dev
Are you sure you want to change the base?
Conversation
Package Changes Through fb52c34There are 1 changes which include tao with minor Planned Package VersionsThe following package releases are the planned based on the context of changes in this pull request.
Add another change file through the GitHub UI by following this link. Read about change files or the docs at github.com/jbolda/covector |
|
needs tauri-apps/wry#1633 |
src/platform_impl/ios/window.rs
Outdated
| // when we support multiple scenes, request the activation of this window's scene | ||
| if application.supportsMultipleScenes() { | ||
| // alternative function is iOS 17+ | ||
| #[allow(deprecated)] |
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.
Since this API is deprecated and will be removed in the future we should use activateSceneSession(for:errorHandler:).
Can we use conditional logic based on availability? e.g.
if #available(iOS 17, *) {
// Use supported API
} else {
#[allow(deprecated)]
// Fallback to deprecated API
}Also - no error handler is set which means scene activation failures will be invisible.
Should we add logging and an optional fallback path?
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.
|
|
||
| // alternative function is iOS 17+ | ||
| #[allow(deprecated)] | ||
| application.requestSceneSessionActivation_userActivity_options_errorHandler( |
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.
Using deprecated API, no error handler is set. See this comment.
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.
|
|
||
| // instead of returning an Option here, we default to an empty string | ||
| // scene lifecycle will be enforced anyway soon (iOS 27) | ||
| pub fn scene_identifier(&self) -> String { |
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.
Multiple windows without scenes get the same "" identifier.
Should we instead return Option<String> and handle downstream?
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.
if you're not using scenes, doesn't make much sense to use this getter
returning an Option here is inconvenient - scenes will be enforced next year, and Apple is already giving warnings
| @@ -143,155 +185,125 @@ impl<T: 'static> EventLoop<T> { | |||
| } | |||
| Event::Stop => self.running = false, | |||
| Event::Start => self.running = true, | |||
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.
Handlers for Event::Start and EventSource::InputQueue have been commented out. Is additional work required?
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.
those variants simply weren't being used
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Leverages scenes on iOS and Activity embedding on Android.
iOS
Android