Adyen / adyen-flutter

MIT License
22 stars 3 forks source link

Cannot style the Card component on the iOS side #200

Closed kamilraczka closed 1 week ago

kamilraczka commented 2 weeks ago

Describe the bug

I am using the Card component from the library. The component itself is working fine on both platforms but the problem comes with the styling itself - on Android I was able to apply custom style but iOS version of the app doesn't reflect the changes I put inside the AdyenAppearance class. I did everything what README.md and your examples say but the changes are not shown in my app.

Below I am providing my research's result also.

To Reproduce

Steps to reproduce the behavior:

  1. Update the AppDelegate.swift as your example shows:
    
    import Adyen
    import adyen_checkout
    import UIKit
    import Flutter

@UIApplicationMain @objc class AppDelegate: FlutterAppDelegate { override func application( _ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]? ) -> Bool { GeneratedPluginRegistrant.register(with: self) return super.application(application, didFinishLaunchingWithOptions: launchOptions) } }

class AdyenAppearance: AdyenComponentAppearanceProvider { static func createCardComponentStyle() -> Adyen.FormComponentStyle { var style = Adyen.FormComponentStyle() style.mainButtonItem.button.backgroundColor = .red style.mainButtonItem.button.title.color = .green return style } }

3. Launch the Flutter app and go to place where you'd place the Card component
4. Changes are not applied

**Expected behavior**

Card component should have a button with the red background and green text but instead it has a default blue button with a white text.

**Desktop (please complete the following information):**

N/A

**Smartphone (please complete the following information):**

 - Device: iPhone 13
 - OS: 17.5.1
 - Browser: Safari
 - Version of the lib: 0.1.0

**Additional context**

After some digging I found that `ConfigurationMapper.swift` and `AdyenAppearance.swift` are the files where problem begins.

When I try to style my card component the `findCardComponentStyle()` method (from `AdyenAppearance.swift`) is being invoked which passes data to the `findAppearanceProvider()` method. This provider should find and return my class I have inside my `AppDelegate.swift` but instead the method always returns the nil value (47th line with `let appearanceProviders [...]` holds a nil after search process). If `findAppearanceProvider()` always returns a nil then `ConfigurationMapper.swift` at 110th line always returns a default instance of `FormComponentStyle` which doesn't include my style changes.

As I mentioned: at the moment `findAppearanceProvider()` method always returns a nil and I think it may be due to the type casting. I did another experiment and made a similar "provider finder" but a typed one, without the generic argument. My `findCardComponentStyle()` method looked like below:
```swift
 static func findCardComponentStyle() -> Adyen.FormComponentStyle? {
        let appearanceProviders = Bundle.allBundles
            .compactMap { $0.infoDictionary?[bundleExecutableKey] as? String }
            .map { $0.replacingOccurrences(of: " ", with: "_") }
            .map { $0.replacingOccurrences(of: "-", with: "_") }
            .compactMap { NSClassFromString("\($0).\(expectedClassName)") }
            .compactMap { $0 as? AdyenComponentAppearanceProvider.Type }

        if let appearanceProvider = appearanceProviders.first {
            return appearanceProvider.createCardComponentStyle()
        }
        else {
            return nil
        }
    }

which is pretty much the same as the original "provider finder" but .compactMap has a strict type. This solution doesn't return the nil value and findCardComponentStyle() method returns me correct FormComponentStyle from my AppDelegate.swift file. I didn't open any Pull Request because I am not sure how much you want to proceed with the generic provider finder.

Bonus info

111th line from ConfigurationMapper.swift always override the background as the white one. I also don't think it should take place on the customisation since the card component then has no possibility to apply other background than white (e.g. in my case I'd love to override the background color to the gray one).

Does all the changes I mentioned above should be classified as the bugs in the library or maybe I am doing something wrong with configuration?

Looking forward to hear your opinion guys and btw: thanks for the library, it really saves time!

Robert-SD commented 1 week ago

Hello @kamilraczka,

Thank you very much for raising this issue and your detailed description. Also for digging into the code and providing additional context. It is very helpful for us to receive feedback so we can improve the SDK further. Especially for adding your bonus info! We are glad to hear that you like the SDK 💚

I will look into the issue and apply your findCardComponentStyle() to verify that it will resolve the problem. On a first glance it might be a bug because your configuration looks fine. But I will try to reproduce it and add a fix. Like you also mentioned, setting the background to white as default shouldn't take place any longer. I will create a PR to change that and keep you updated.

Robert-SD commented 1 week ago

Hi @kamilraczka, I would like to provide a quick update. As mentioned, I've adjusted the appearance providers and moved to the strict type. I tested your configuration and it seems to work. Would it be possible for you testing the branch feature/improveAppearanceProvider as well? Your feedback would be great.

Simulator Screenshot - iPhone

Robert-SD commented 1 week ago

Hi @kamilraczka, I would like to provide a short follow up. We decided to improve styling experience by simplifying the setup. The AdyenAppearance enum is now exposed. You can just override the style and it will be applied. Implementing the AdyenComponentAppearanceProvider interface is no longer required. Feel free to have a look into our example and in case of questions, please reopen this issue.

kamilraczka commented 6 days ago

Hello @Robert-SD!

First of all, sorry for my late response - I didn't check the previous changes on the branch you have linked but I am going to try the new ones. In case of any problems I am going to let you know. Thanks for quick actions though!