Skip to content
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

[Feature Request] Provide a Dart Idiomatic Async API #890

Open
caseycrogers opened this issue Jul 21, 2023 · 6 comments
Open

[Feature Request] Provide a Dart Idiomatic Async API #890

caseycrogers opened this issue Jul 21, 2023 · 6 comments
Labels
feature request Feature request covers a product enhancement

Comments

@caseycrogers
Copy link

caseycrogers commented Jul 21, 2023

[REQUIRED] Step 2: Describe the problem

The entire google_mobile_ads API is built with callback APIs. These are bug prone and very non-idiomatic for Flutter. Really non-idiomatic in any language post ~2005. This means each dev more or less needs to write a wrapper around the current API to convert it to Flutter idiomatic futures.

Here is an example of what the API would look like using Flutter/Dart idioms:

// Future allows the dev to use `FutureBuilder` instead of customized stateful logic.
Future<RewardedAd> ad = RewardedAd.load();

// Now we don't have to navigate several different callbacks.
// The stream will complete with either an earned or 
// dismissed event to indicate the outcome of showing the
// ad.
Stream<RewardedEvent> result = (await ad).show();

// Class that signals events while the ad is up.
// Subclasses could include things like:
//    RewardEarned(Reward reward);
//    AdDismissed(DismissDetails details);
abstract class RewardedAdEvent { ... }

However even this isn't very Flutter idiomatic. It really isn't even going far enough. The ideal would be a widget using the controller pattern roughly as follows (see ScrollController and Scrollable for full Flutter idiomatic examples of the controller pattern):

// Wrap a widget subtree in this to allow it to show ads.
class RewardedAdView extends StatefulWidget {
  static RewardedAdController of(BuildContext context) { ... }

  // Can be passed in the constructor or makes a default one.
  RewardedAdController controller;
  ...
}

class _RewardedAdView extends State<RewardedAdView> {
  Widget build(BuildContext context() {
    // Wraps child in a provider so that the subtree can call `.of`.
    return _RewardedAdControllerProvider(controller: widget.controller, ...);
  }
}

class _RewardedAdControllerProvider extends InheritedWidget {
  RewardedAdController controller;
  ...
}

// Manages current ad state.
class RewardedAdController  with ChangeNotifier {
  RewardedAd? ad;

  // Sets `ad`. Could possibly be private and just called
  // by the view class's `initState` method.
  Future<void> load() { ... }

  // Show the ad and get the results stream.
  Stream<RewardedAdEvent> show() async* { ... };
}

// Somewhere in a build function.
return MyButton(
  child: Text('Oh yeah! Show me an ad, I love ads!'),
  onPressed: () {
    final result = RewardedAdView.of(context).show();
    ...
  },
);
@huycozy huycozy added in triage Issue currently being evaluated feature request Feature request covers a product enhancement and removed in triage Issue currently being evaluated labels Jul 24, 2023
@huycozy
Copy link
Collaborator

huycozy commented Jul 25, 2023

I think this is a wide scope issue and may need a lot of codebase refactoring, not sure about the feasibility and amount of effort required for now. So, I'm labeling this with a single label for more information from others.

@mulderpf
Copy link

This would be a welcome change!!! We already experienced a huge amount of refactoring from the 0. versions of the plugin until today, so anything that is more like what we are used to using everyday, will likely cause a lot less issues. The way to use the plugin is a bit weird and doesn't feel like the rest of my code.

@caseycrogers
Copy link
Author

Any news here? The existing API is really hard to use.

@caseycrogers
Copy link
Author

I'm now adding Banner Ads to my app and am reminded how hilariously hard to use the AdMob APIs are:
https://codelabs.developers.google.com/codelabs/admob-ads-in-flutter#6

@override
void initState() {
  ...

  // Why are we creating an instance if we're not going to hold a reference to it??
  BannerAd(
    adUnitId: AdHelper.bannerAdUnitId,
    request: AdRequest(),
    size: AdSize.banner,
    // Why is a simple callback it's own class??????
    listener: BannerAdListener(
      onAdLoaded: (ad) {
        setState(() {
          // WHHHHHYYYYYY do we have to do type casting here?????
          _bannerAd = ad as BannerAd;
        });
      },
      onAdFailedToLoad: (ad, err) {
        print('Failed to load a banner ad: ${err.message}');
        ad.dispose();
      },
    ),
  ).load();
}

This feels like 2010s "Enterprise grade" Java or something. This is really painful. I hope you guys fix it.

@malandr2
Copy link
Collaborator

malandr2 commented Oct 2, 2023

@caseycrogers this is something we are actively looking into but no immediate news to share. However, comments like these show the importance of this feature request. Thanks

@rajabilal555
Copy link

I have done this using simple completers. If someone wants inspiration for using this api.
Maybe these methods can be a part of the sdk one day...

Future<RewardedInterstitialAd> _populateRewardedInterstitialAd({
    required String adUnitId,
    required VoidCallback onAdDismissedFullScreenContent,
    required VoidCallback onAdShowedFullScreenContent,
  }) async {
    try {
      final adCompleter = Completer<Ad?>();
      await RewardedInterstitialAd.load(
        adUnitId: adUnitId,
        request: const AdRequest(),
        rewardedInterstitialAdLoadCallback: RewardedInterstitialAdLoadCallback(
          onAdLoaded: (ad) {
            ad.fullScreenContentCallback = FullScreenContentCallback(
              onAdShowedFullScreenContent: (ad) {
                onAdShowedFullScreenContent();
              },
              onAdDismissedFullScreenContent: (ad) {
                onAdDismissedFullScreenContent();
              },
            );

            adCompleter.complete(ad);
          },
          onAdFailedToLoad: adCompleter.completeError,
        ),
      );

      final rewardedInterstitial = await adCompleter.future;
      if (rewardedInterstitial == null) {
        throw const AdsClientException('Rewarded Interstitial Ad was null');
      }
      return rewardedInterstitial as RewardedInterstitialAd;
    } catch (error, stackTrace) {
      Error.throwWithStackTrace(AdsClientException(error), stackTrace);
    }
  }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Feature request covers a product enhancement
Projects
None yet
Development

No branches or pull requests

5 participants