MaikuB / flutter_appauth

A Flutter wrapper for AppAuth iOS and Android SDKs
269 stars 239 forks source link

[flutter_appauth] iOS 13 Support For Multiple Windows #361

Closed jeroldalbertson-wf closed 1 year ago

jeroldalbertson-wf commented 1 year ago

With Multi-window support introduced in iOS 13, for apps that enable this feature one can no longer grab the rootViewController from the AppDelegate's window, instead it should come from the keyWindow for the active scene.

MaikuB commented 1 year ago

Thanks for the PR. The changes make sense but I don't use much of iOS these days so do you have any advice on how to see what difference these changes will make or is this more of a code thing? If it's the latter, for my own benefit in upskilling since I'm not up to date on the iOS changes, do you happen have any links to resources on how to handle these situations? To re-emphasise I'm asking due to the lack of knowledge on my part. It's not because I don't believe these changes are required as logically they make sense

astinka commented 1 year ago

I'd like to bump this PR because without it you will get the following error when using the plugin with a SceneDelegate on iOS 13+ : presentingViewController cannot be nil on iOS 13 I'm sorry that my Swift knowledge isn't good enough to explain in detail, but with this fix, it is working when the iOS-application is using multiple scenes.

MaikuB commented 1 year ago

@astinka thanks for sharing. The main reason why the PR hasn't been merged is I haven't heard back on how to test this. Do you think you could share a link to a repository with an app that uses the changes in this PR? Since you mention using a SceneDelegate, this would imply you're using an app with multiple window support. However, I'm not across what is needed to implement a Flutter application with multiple window support and I suspect this requires each Window to have a separate instance of the Flutter engine.

astinka commented 1 year ago

@MaikuB I have set up a sample project that uses multiple scenes and a SceneDelegate to reproduce the error here:

https://github.com/astinka/flutter_appauth_scene_delegate

Please take note that the fix by @jeroldalbertson-wf also needs to be applied to the performEndSessionRequest in the same file.

MaikuB commented 1 year ago

@astinka thanks for providing this. Was able to reproduce this though my method was to copy the main.dart file from the plugin's example app over as I don't have Keycloak set up where it looks like your code was setup for Keycloak. I made the changes to the end session request too. Everything seems fine now but could you help test to this if there's any other issue before I convert this draft PR to be ready so that it can be merged in?

dballance commented 1 year ago

I have tested this in our fork - and am actively using it to fix an issue with our use of SceneDelegate.

Would be great to pull this in so we don't have to override this dep.