googleads / googleads-mobile-ios-mediation

Apache License 2.0
120 stars 137 forks source link

Added support for bidding native ads. #459

Closed hikaruwatanabe-eng closed 4 months ago

hikaruwatanabe-eng commented 4 months ago

Added support for bidding native ads.

google-cla[bot] commented 4 months ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

phenan commented 4 months ago

[question]

Is it okay for GADMediationAdapterLineNativeAdLoader API to ignore extras.nativeAdVideoWidth if it is specified? If you need to consider extras.nativeAdVideoWidth, I think you need to resize the view obtained with getAdMainView appropriately.


GADMediationAdapterLineNativeAdLoader API は extras.nativeAdVideoWidth が指定されていた場合にそれを無視しても良いのでしょうか? もし extras.nativeAdVideoWidth を考慮する必要がある場合は、getAdMainView で取得したビューを適切にリサイズする必要があると思います

hikaruwatanabe-eng commented 4 months ago

[question]

Is it okay for GADMediationAdapterLineNativeAdLoader API to ignore extras.nativeAdVideoWidth if it is specified? If you need to consider extras.nativeAdVideoWidth, I think you need to resize the view obtained with getAdMainView appropriately.

GADMediationAdapterLineNativeAdLoader API は extras.nativeAdVideoWidth が指定されていた場合にそれを無視しても良いのでしょうか? もし extras.nativeAdVideoWidth を考慮する必要がある場合は、getAdMainView で取得したビューを適切にリサイズする必要があると思います

That was my question too. Thank you the clarification! I will make an update.

hikaruwatanabe-eng commented 4 months ago

Updated the code. The extras.nativeAdViewoWidth was added before because FiveAd needed the parameter in the older API.

 [[FADNative alloc] initWithSlotId:slotID videoViewWidth:extras.nativeAdVideoWidth];

But now the ad loader doesn't take it, I just resized the native ad's main view as suggested. If you think it needs to be done other way, please let me know.

hikaruwatanabe-eng commented 4 months ago

The new API does not have width parameter as you know, we don't know this design choice is good or bad.

From Google's perspective, do you think it's better to take the width explicitly?

If it is better to take width parameter, it is possible to release a version of the SDK with improved API within a few days.

新しい AdLoader API は width を取らない形式になっていますが、このデザインが良いかどうかはまだ悩んでいる段階です。

Google からみて、width は引数に取るべきだと思うでしょうか?

もしそうであれば、数日以内に width を引数に取るバージョンの SDK をリリースする用意があります。

Either works, but I think it would be better for LINE SDK to accept the width parameter so that in the future, if this API is used by anyone other than the adapter, LINE would have control over how to set the width. This way, the ad sizing logic would most likely be consistent across different API consumers. But ultimately, the decision is yours. :)

phenan commented 4 months ago

Either works, but I think it would be better for LINE SDK to accept the width parameter so that in the future, if this API is used by anyone other than the adapter, LINE would have control over how to set the width.

Thank you for your kindly response. I will reconsider the API within my team.

We will contact you again if we change the API. (We do not intend to make any breaking changes to the current API)