FirebaseUI-iOS icon indicating copy to clipboard operation
FirebaseUI-iOS copied to clipboard

Activity indicator appearing in custom login view controller when using Facebook, Twitter or Google login.

Open warren-gavin opened this issue 8 years ago • 7 comments

  • Objective C or Swift: Objective C
  • iOS version: 10+
  • Firebase SDK version: 4.0.0
  • FirebaseUI version: 4.0.0
  • CocoaPods Version: 1.2.0

Describe the problem:

Commit 3ba8a76 introduced

    + (UIActivityIndicatorView *)addActivityIndicator:(UIView *)view;

to the FUIAuthBaseViewController class. Each of the Facebook, Twitter and Google authentication providers use this method to add an activity indicator to the presenting controller.

However, this does not take custom login view controllers into consideration, which have their own UI and UX.

Steps to reproduce:

  1. Create a login flow with a custom login view controller, which inherits from UIViewController and not FUIAuthBaseViewController
  2. Enable Facebook, Twitter or Google login
  3. Login via one of those Auth providers.

Observed Results:

  • The auth provider will spawn a modal webview for the auth provider's authentication steps
  • Once authenticated you are returned to the original login view controllers
  • This view controller will display a black activity indicator

Expected Results:

  • As the login view controller is custom, the UI library should not add any subviews to it.

warren-gavin avatar Nov 26 '17 20:11 warren-gavin

We recently exposed the incrementActivity and decrementActivity, which you can override in your custom subclass to disable adding or removing the activity indicator.

morganchen12 avatar Aug 30 '18 20:08 morganchen12

@morganchen12 Sorry for not getting back to this sooner, but I'm afraid decrementActivity will only work if the custom login view controller inherits from FUIAuthBaseViewController, which isn't necessarily the case.

I have made an example login controller here: https://github.com/warren-gavin/APAccessibleFirebaseLogin

This is a vanilla login view controller that inherits from UIViewController and uses Google as an authentication provider. FUIGoogleAuth assumes all login view controllers are FUIAuthBaseViewController subclasses so calls FUIAuthBaseViewController. addActivityIndicator directly, and it's not possible to call decrementActivity on a vanilla view controller.

I believe the fix would be to check if the _presentingViewController's class in the auth provider responds to the addActivityIndicator static method. I would be happy to create a PR for this if you agree.

warren-gavin avatar Jan 21 '19 10:01 warren-gavin

I see. Is there a reason you're specifically not using FUIAuthBaseViewController? Adding vanilla support may be more complex than just checking the class, since the xib file assumes it's a controller of type FUIAuthBaseViewController.

Other than that, I'm fine with adding support for regular UIViewControllers.

morganchen12 avatar Jan 22 '19 18:01 morganchen12

I may have misunderstood the intent of FUIAuthBaseViewController and how it can be extended, but the documentation stated that I could customise everything except for the actual sign-in buttons.

My app uses dynamic type to support larger text sizes for accessibility, so not being able to modify the buttons, which have fixed size fonts, was an issue for me. I created my own buttons, sized as I require them and using dynamic type, and they use the various concrete instances ofFUIAuthProvider under the hood to perform sign-in

warren-gavin avatar Jan 22 '19 22:01 warren-gavin

Filed #597 to address the lack of dynamic type support. Looks like the only workaround for now (for the behavior you described at the beginning of this issue) is to swizzle the addActivityIndicator: method and make it do nothing.

In the future, I'm hoping to expose the button construction/setup methods so they can be overridden for easier customization as well.

morganchen12 avatar Jan 22 '19 23:01 morganchen12

Thanks @morganchen12

Indeed, the workaround I have implemented in my project has been to swizzle addActivityIndicator

warren-gavin avatar Jan 23 '19 07:01 warren-gavin

In case anyone is wondering what @warren-gavin is talking about, I've also added the following as a function in my AppDelegate, which I call at the end of application(application:didFinishLaunchingWithOptions:)

func swizzleFUISpinner () {
        
    let originalMethod = class_getClassMethod(FUIAuthBaseViewController.self, #selector(FUIAuthBaseViewController.addActivityIndicator(_:)))
    let swizzledMethod = class_getClassMethod(SwizzleClass.self, #selector(SwizzleClass.fake_addAlert(_:)))
    if let originalMethod = originalMethod, let swizzledMethod = swizzledMethod {    
        method_exchangeImplementations(originalMethod, swizzledMethod)
    }
}

then my SwizzleClass looks like this

fileprivate class SwizzleClass: NSObject {
    @objc class func fake_addAlert (_ view: UIView) -> UIActivityIndicatorView {
        print("no alerts here pls")
        return UIActivityIndicatorView(frame: .zero)
    }
}

This is the only way I could get rid of that default spinner.

mylogon341 avatar Aug 28 '19 23:08 mylogon341