ionic-team / capacitor

Build cross-platform Native Progressive Web Apps for iOS, Android, and the Web ⚡️
https://capacitorjs.com
MIT License
12.26k stars 1.01k forks source link

fix(ios): Make Bridge webView first responder #7753

Closed theproducer closed 22 hours ago

theproducer commented 2 weeks ago

Fixes a focus issue (reproduction) caused when the webView is not the first responder. CAPBridgeViewController's self.becomeFirstResponder() was being called during viewDidLoad, before the view's window is set, which had no effect.

jcesarmobile commented 1 week ago

Just remembered this initialFocus configuration option we added for Android, maybe we should add it for iOS too?

https://github.com/ionic-team/capacitor/pull/5579

markemer commented 1 week ago

Using self.webView?.becomeFirstResponder() in the same line were self.becomeFirstResponder() was also seems to work fine without the addition of the viewDidAppear override.

Rethinking this comment, the viewDidAppear method will be called in apps that embed the CAPBridgeViewController.swift whenever it appears, so putting the line there will make the WebView to regain focus, so looks like a better place than viewDidLoad.

@Steven0351 thoughts?

Yeah - viewDidAppear is probably better - but, I don't want to break Portal, et. al.

Steven0351 commented 1 week ago

Using self.webView?.becomeFirstResponder() in the same line were self.becomeFirstResponder() was also seems to work fine without the addition of the viewDidAppear override. Rethinking this comment, the viewDidAppear method will be called in apps that embed the CAPBridgeViewController.swift whenever it appears, so putting the line there will make the WebView to regain focus, so looks like a better place than viewDidLoad. @Steven0351 thoughts?

Yeah - viewDidAppear is probably better - but, I don't want to break Portal, et. al.

Nothing in here would affect Portals at all, so it's all good 👍, +1 for viewDidAppear

jcesarmobile commented 1 week ago

Not portals but embedded Capacitor, not sure what his name is nowadays.

I think this should be behind a initialFocus configuration option like on Android and also make a common initialFocus configuration option for both platforms as we do for some other settings so can be set globally or per platform, but the common one could be kept out of this PR since this is about iOS.