-
Notifications
You must be signed in to change notification settings - Fork 3.5k
[interactive_media_ads] Adds support for companion ads #9260
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
Conversation
packages/interactive_media_ads/lib/src/ios/ios_companion_ad_slot.dart
Outdated
Show resolved
Hide resolved
packages/interactive_media_ads/lib/src/ios/ios_companion_ad_slot.dart
Outdated
Show resolved
Hide resolved
|
|
||
| /// The height of the companion slot. | ||
| /// | ||
| /// Returns `null` when [isFluid] returns true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Having this be comment-enforced rather than type-enforced isn't ideal; all the uses of these end up doing force-unwraps. What about using a sealed class called something like CompanionAdSize, with subclasses CompanionAdSizeFixed and ...Fluid, where the former has non-nullable width and height fields, and the latter is empty? Then this class can have a non-nullable size (instead of width, height, and isFluid), and usage will get full type safety as to whether or not there is a width and height.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good suggestion. I added the lib/src/platform_interface/ad_companion_ad_slot_size.dart file with the sealed classes.
stuartmorgan-g
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
flutter/packages@0b322a2...6d3aaf4 2025-06-11 [email protected] Roll Flutter master to 824868f (flutter/packages#9416) 2025-06-11 [email protected] [interactive_media_ads] Adds support for companion ads (flutter/packages#9260) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes flutter/flutter#153446 Adds `CompanionAdSlot` and `AdDisplayContainer(companionAds)`. ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
…r#170528) flutter/packages@0b322a2...6d3aaf4 2025-06-11 [email protected] Roll Flutter master to 824868f (flutter/packages#9416) 2025-06-11 [email protected] [interactive_media_ads] Adds support for companion ads (flutter/packages#9260) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Fixes flutter/flutter#153446 Adds `CompanionAdSlot` and `AdDisplayContainer(companionAds)`. ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Fixes flutter/flutter#153446
Adds
CompanionAdSlotandAdDisplayContainer(companionAds).Pre-Review Checklist
[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3