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

feat(ios, android): add FLUID size support #556

Merged
merged 14 commits into from
Apr 3, 2024

Conversation

Almouro
Copy link
Contributor

@Almouro Almouro commented Mar 19, 2024

Description

Support for FLUID size on Android and fix for iOS FLUID size.

I've built onto this PR to make sure we didn't have to set a fixed height to the ads.
We set "width: 100%" and the ads are responsible for setting their own height

Easier to check the whole changes as a whole with whitespace hidden (https://github.com/invertase/react-native-google-mobile-ads/pull/556/files?diff=split&w=1)

Related issues

Android: FLUID size wasn't supported yet on Android. Fixes #120
iOS: FLUID size was taking full device width. Fixes #213

Supercedes / Closes #277

Release Summary

feat: add FLUID size support

Checklist

  • I read the Contributor Guide
    and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in __tests__e2e__
    • jest tests added or updated in __tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

I've used the example app to test my changes (as well as a client app with actual production banners)

  • On iOS: tested on Iphone 15 simulator iOS 17.0
  • On Android: tested on
    • Samsung Galaxy A10s Android 10
    • Pixel 8 Android 14
iOS A10s Pixel 8
image image image

I've also checked the other banners on Android, since they're affected by this change

Think react-native-google-mobile-ads is great? Please consider supporting the project with any of the below:

  • 👉 Star this repo on GitHub ⭐️
  • 👉 Follow Invertase on Twitter

🔥

@CLAassistant
Copy link

CLAassistant commented Mar 19, 2024

CLA assistant check
All committers have signed the CLA.

@Almouro Almouro marked this pull request as ready for review March 19, 2024 09:33
@dylancom dylancom requested a review from mikehardy March 22, 2024 13:05
Copy link

codecov bot commented Mar 26, 2024

Codecov Report

Merging #556 (19377a8) into main (8dd5b93) will increase coverage by 0.06%.
The diff coverage is 40.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #556      +/-   ##
==========================================
+ Coverage   43.66%   43.72%   +0.06%     
==========================================
  Files          29       30       +1     
  Lines         536      549      +13     
  Branches      147      151       +4     
==========================================
+ Hits          234      240       +6     
- Misses        302      309       +7     

@Almouro
Copy link
Contributor Author

Almouro commented Mar 27, 2024

When using AdManager and passing multiple sizes including FLUID instead of a single size, I realized that it was still not working properly
Added a commit to address that

I find it quite useful especially for fluid ads which don't necessarily have their proper size set once loaded
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite a chunk of work, thank. you! It all looks reasonable to me

@dylancom dylancom merged commit c09a326 into invertase:main Apr 3, 2024
10 of 13 checks passed
@mikehardy
Copy link
Collaborator

🎉 This PR is included in version 13.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[🐛] Fluid size not correctly Extend AdSize support to handle size=FLUID
6 participants