Conversation
Replace @no banner stubs with full native implementation matching Android behavior. Banners are created via CloudXCore API, positioned using Auto Layout constraints against the safe area, and managed through show/hide/destroy lifecycle with delegate callbacks sending events back to Dart via the method channel. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Code ReviewOverall this looks solid — the implementation closely follows both the Android plugin and AppLovin's iOS patterns. A few items to consider: Minor issues
// .h
@property (nonatomic, strong, readonly) FlutterMethodChannel *channel;
// .m class extension
@property (nonatomic, strong, readwrite) FlutterMethodChannel *channel;
Future considerationThe delegate callbacks hardcode |
There was a problem hiding this comment.
Pull request overview
This PR implements native iOS banner ad support for the CloudX Flutter SDK, replacing stub methods that returned @NO with a complete implementation using the CloudXCore native SDK. The implementation adds banner lifecycle management (create, show, hide, destroy), programmatic positioning via Auto Layout with safe area support, delegate-based event handling, and proper resource cleanup.
Changes:
- Added full banner ad lifecycle implementation with CloudXCore SDK integration, including programmatic positioning via Auto Layout constraints
- Implemented CLXBannerDelegate and CLXAdRevenueDelegate callbacks to send ad events to Dart via method channel with Android-compatible event format
- Added show-before-create queuing mechanism and auto-refresh management to prevent wasted impressions when banners are hidden
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| cloudx_flutter_sdk/ios/Classes/CloudXFlutterSdkPlugin.h | Added CloudXCore import and protocol conformance (CLXBannerDelegate, CLXAdRevenueDelegate), exposed channel property |
| cloudx_flutter_sdk/ios/Classes/CloudXFlutterSdkPlugin.m | Implemented complete banner ad support: lifecycle methods, Auto Layout positioning, delegate callbacks, event serialization, and resource management |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return; | ||
| } | ||
|
|
||
| CLXBannerAdView *adView = [[CloudXCore shared] createBannerWithAdUnitId:adUnitId viewController:rootViewController]; |
There was a problem hiding this comment.
The createBannerWithAdUnitId:viewController: method is being called without checking if CloudXCore is initialized first. According to the initialization pattern in lines 52-62, CloudXCore needs to be initialized before creating ad views. Consider adding a guard to check if CloudXCore.shared.isInitialized is true, and handle the case where the SDK hasn't been initialized yet (e.g., log an error and return early).
| - (void)didLoadAd:(CLXAd *)ad { | ||
| NSString *adUnitId = ad.adUnitId; | ||
| NSLog(@"[CloudX Flutter] Banner loaded: %@", adUnitId); | ||
|
|
||
| CLXBannerAdView *adView = self.adViews[adUnitId]; | ||
|
|
||
| // Re-position after load in case the ad view's size changed | ||
| [self positionAdViewForAdUnitIdentifier:adUnitId position:self.adViewPositions[adUnitId]]; | ||
|
|
||
| // Do not auto-refresh if the ad view is not showing yet (e.g. first load before publisher shows banner). | ||
| // Auto-refresh will resume when showBanner is called. | ||
| if (adView && adView.hidden) { | ||
| [adView stopAutoRefresh]; | ||
| } | ||
|
|
||
| [self sendEvent:@"OnBannerAdLoadedEvent" adUnitId:adUnitId data:@{@"ad": [self serializeAd:ad]}]; | ||
| } | ||
|
|
||
| - (void)didFailToLoadAd:(NSString *)adUnitId error:(CLXError *)error { | ||
| NSLog(@"[CloudX Flutter] Banner load failed: %@ - %@", adUnitId, error.message); | ||
| [self sendEvent:@"OnBannerAdLoadFailedEvent" adUnitId:adUnitId data:@{@"error": [self serializeError:error]}]; | ||
| } | ||
|
|
||
| - (void)didClickAd:(CLXAd *)ad { | ||
| NSString *adUnitId = ad.adUnitId; | ||
| NSLog(@"[CloudX Flutter] Banner clicked: %@", adUnitId); | ||
| [self sendEvent:@"OnBannerAdClickedEvent" adUnitId:adUnitId data:@{@"ad": [self serializeAd:ad]}]; | ||
| } | ||
|
|
||
| - (void)didExpandAd:(CLXAd *)ad { | ||
| NSString *adUnitId = ad.adUnitId; | ||
| NSLog(@"[CloudX Flutter] Banner expanded: %@", adUnitId); | ||
| [self sendEvent:@"OnBannerAdExpandedEvent" adUnitId:adUnitId data:@{@"ad": [self serializeAd:ad]}]; | ||
| } | ||
|
|
||
| - (void)didCollapseAd:(CLXAd *)ad { | ||
| NSString *adUnitId = ad.adUnitId; | ||
| NSLog(@"[CloudX Flutter] Banner collapsed: %@", adUnitId); | ||
| [self sendEvent:@"OnBannerAdCollapsedEvent" adUnitId:adUnitId data:@{@"ad": [self serializeAd:ad]}]; | ||
| } | ||
|
|
||
| // ============================================================================ | ||
| #pragma mark - CLXAdRevenueDelegate | ||
| // ============================================================================ | ||
|
|
||
| - (void)didPayRevenueForAd:(CLXAd *)ad { | ||
| NSString *adUnitId = ad.adUnitId; | ||
| NSLog(@"[CloudX Flutter] Banner revenue paid: %@", adUnitId); | ||
| [self sendEvent:@"OnBannerAdRevenuePaidEvent" adUnitId:adUnitId data:@{@"ad": [self serializeAd:ad]}]; |
There was a problem hiding this comment.
The delegate methods hardcode "OnBannerAd*Event" for all events. When MREC support is added in the future (currently stubbed at lines 105-116), these same delegate methods will also be called for MREC ads since both use CLXBannerDelegate. This will cause MREC events to be incorrectly sent as banner events, routing them to the wrong listener in Dart. Consider adding a mechanism to track the ad type (e.g., store format type alongside the adView, or check ad.adFormat) so the correct event prefix can be used, similar to the Android pattern at CloudXFlutterSdkPlugin.kt:813-817 which uses "On${adFormat}AdLoadedEvent".
| [eventData addEntriesFromDictionary:data]; | ||
| } | ||
|
|
||
| [self.channel invokeMethod:eventName arguments:eventData]; |
There was a problem hiding this comment.
When calling invokeMethod on the Flutter channel at line 494, there's no check to ensure self.channel is not nil. If the plugin is being torn down or the channel hasn't been set up yet, this could cause a crash. Consider adding a nil check for self.channel before invoking the method, or ensure initialization order guarantees the channel is always available when delegate methods are called.
| - (NSDictionary *)serializeAd:(CLXAd *)ad { | ||
| return @{ | ||
| @"adUnitId": ad.adUnitId ?: @"", | ||
| @"adFormat": ad.adFormat ?: @"", |
There was a problem hiding this comment.
The adFormat field is not being lowercased, but the Android implementation lowercases it using ad.adFormat.name.lowercase() at CloudXFlutterSdkPlugin.kt:769. This inconsistency could cause issues in the Dart layer if it expects a specific format. Consider lowercasing the adFormat value to match the Android implementation, for example: adFormat.lowercaseString or ensuring the CloudXCore SDK returns it in the expected format.
| @"adFormat": ad.adFormat ?: @"", | |
| @"adFormat": ad.adFormat ? [ad.adFormat lowercaseString] : @"", |
| if ([position hasSuffix:@"_left"]) { | ||
| [constraints addObject:[adView.leftAnchor constraintEqualToAnchor:superview.leftAnchor]]; | ||
| } else if ([position hasSuffix:@"_right"]) { | ||
| [constraints addObject:[adView.rightAnchor constraintEqualToAnchor:superview.rightAnchor]]; | ||
| } else { | ||
| // top_center, bottom_center, centered | ||
| [constraints addObject:[adView.centerXAnchor constraintEqualToAnchor:safeArea.centerXAnchor]]; | ||
| } |
There was a problem hiding this comment.
There's an inconsistency in the anchor usage for positioning. Vertical positioning uses safeArea anchors for all cases (lines 384, 386, 389), but horizontal positioning uses superview anchors for left/right (lines 395, 397) and safeArea for center (line 400). For left/right positioning, consider using safeArea anchors to respect safe area insets (e.g., notches, rounded corners), or use superview anchors consistently if the intention is to ignore safe areas for horizontal edges. The comment at line 392-393 suggests physical positioning is desired, but this should still respect safe areas to avoid content being clipped by device features.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unnecessary nil guard on adFormat (nonnull) - Fix revenue: NSNumber* nullable, not primitive — use ?: @0 - Fix error.message → error.localizedDescription (no message property on CLXError) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Use umbrella header CloudXCore.h (resolves module import for delegates) - Convert CLXAdFormat enum to string for NSDictionary serialization - Fix error.message → error.localizedDescription in NSLog Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Without explicit width/height constraints, Auto Layout collapses the view to zero size — ad renders visually but touches pass through. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…hannel - Use static singleton to preserve ad state across Flutter hot restarts - Use adEventPrefixes dictionary for format-aware event names (Banner/MRec/etc) - Remove unused FlutterStreamHandler and EventChannel scaffolding Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Summary
@NObanner stubs in the iOS plugin with full native implementation using CloudXCore APIsafeAreaLayoutGuide(vertical) and physicalleft/rightanchors (horizontal), matching AppLovin's iOS patternCLXBannerDelegateandCLXAdRevenueDelegatecallbacks to send events (OnBannerAd*Event) back to Dart via the method channel, matching the existing Android event formatNSNullfor nullable parameters (placement,customData)Test plan
flutter analyzepasses cleanflutter runon iOS simulator — create banner, verify it renders at correct positiontop_left,top_center,top_right,center_left,centered,center_right,bottom_left,bottom_center,bottom_rightupdateBannerPositionmoves the bannerdestroyBannercleans up the view and constraintssetBannerPlacement(null)doesn't crash (NSNull guard)🤖 Generated with Claude Code