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

Combined iOS and Android ad experience controls documentation into a single file #5150

Conversation

charles-the-developer
Copy link
Contributor

@charles-the-developer charles-the-developer commented Feb 9, 2024

I have implemented tabs to merge the iOS and Android ad experience controls documentation into a single file combined-ad-experience-controls.md.

I have a couple of questions/notes:

  1. On line 16, there is a note saying, "NOTE: Planned future enhancements will support Server Side Configuration. Follow this feature request for the details." Is this note still relevant? The GitHub issue it links to is dated Mar 18, 2022, and the last comment on the issue was made on Aug 11, 2022. Should I keep this note or remove it?
  2. On lines 145-176 under Skip Button Area, the images folder doesn't have a screenshot for iOS.
  3. I have not yet added the merged page to the navigation menu. If you are running the site on localhost, you can access the page via - http://127.0.0.1:8080/prebid-mobile/modules/rendering/combined-ad-experience-controls.html

Copy link

netlify bot commented Feb 9, 2024

Deploy Preview for prebid-docs-preview ready!

Name Link
🔨 Latest commit d6098b1
🔍 Latest deploy log https://app.netlify.com/sites/prebid-docs-preview/deploys/65d65e2f94df6e0008f57ba7
😎 Deploy Preview https://deploy-preview-5150--prebid-docs-preview.netlify.app/prebid-mobile/modules/rendering/combined-ad-experience-controls
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@muuki88
Copy link
Contributor

muuki88 commented Feb 11, 2024

Thanks @charles-the-developer . We also have a preview link automatically created for each PR 😃

https://deploy-preview-5150--prebid-docs-preview.netlify.app/prebid-mobile/modules/rendering/combined-ad-experience-controls

See #5150 (comment)

For the other two questions, can you help @YuriyVelichkoPI ?

Copy link
Contributor

@bretg bretg left a comment

Choose a reason for hiding this comment

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

Love it. Thanks @charles-the-developer . I think we should remove the separate files and change the left-nav links to point to the combined file.


If you use Prebid SDK to render the winning bid you can customize behaviour using the following API.

> NOTE: Planned future enhancements will support Server Side Configuration. Follow this [feature request](https://github.com/prebid/prebid-server/issues/2186) for the details.
Copy link
Contributor

Choose a reason for hiding this comment

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

@YuriyVelichkoPI - I assume this is the bidrequest.ext.prebid.passthrough feature? Is there anything in this doc that should change as a result of this being done in PBS now besides removing this note?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @bretg ! Yes it is about bidrequest.ext.prebid.passthrough. And yes - the doc should contain config keys to change the properties via passthrough. Missing this info in the documentation is our gap. I'll create a ticket to highlight the changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically, the properties are already in the doc. They are listed in the Server Property row for each section. But we still should remove (pending for PBS implementation) text for them.

Copy link
Contributor

@YuriyVelichkoPI YuriyVelichkoPI left a comment

Choose a reason for hiding this comment

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

Just for your info. It is not common for mobile SDK docs to merge functional documentation. As usual, documentation for iOS and Android lives in separate sections. Even if it is super similar. Maybe it because developers don't like to jump over their scope.

Examples:

MAX:
Android: https://dash.applovin.com/documentation/mediation/android/getting-started/integration
iOS: https://dash.applovin.com/documentation/mediation/ios/getting-started/integration

GMA SDK (both for AdMob and GAM integration):
Android: https://developers.google.com/ad-manager/mobile-ads-sdk/android/quick-start
iOS: https://developers.google.com/ad-manager/mobile-ads-sdk/ios/quick-start

ironSource
Android: https://developers.is.com/ironsource-mobile/android/getting-started-android/
iOS: https://developers.is.com/ironsource-mobile/ios/getting-started-ios/

Smaato
Android: https://developers.smaato.com/publishers/nextgen-sdk-android-getting-started/
iOS: https://developers.smaato.com/publishers/nextgen-sdk-ios-getting-started/

All of these don't have common pages. Especially pages with both iOS and Android code together.

So such changes will be unusual for mobile developers.


If you use Prebid SDK to render the winning bid you can customize behaviour using the following API.

> NOTE: Planned future enhancements will support Server Side Configuration. Follow this [feature request](https://github.com/prebid/prebid-server/issues/2186) for the details.
Copy link
Contributor

Choose a reason for hiding this comment

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

Basically, the properties are already in the doc. They are listed in the Server Property row for each section. But we still should remove (pending for PBS implementation) text for them.

@bretg
Copy link
Contributor

bretg commented Feb 21, 2024

Just for your info. It is not common for mobile SDK docs to merge functional documentation.

Yuriy and I discussed this. We agreed that certain pages (e.g the general integration guides for each platform like https://docs.prebid.org/prebid-mobile/pbm-api/ios/code-integration-ios.html) need to be standalone, but that the "recipe" pages can be combined.

Copy link
Contributor

@bretg bretg left a comment

Choose a reason for hiding this comment

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

I updated the PR:

  • removed the previously separate files
  • added link to the left nav
  • removed the "pending" note

Note that the side effect of the combined file is that BOTH iOS and Android left nav entries are opened when looking at this file in preview. I think that's ok.

@YuriyVelichkoPI brought up that really the image examples should have an Android version and an iOS version. It's hard for me to see the difference in the currently live pages so I'm considering this a low priority.

I'm good to merge this with the understanding that @YuriyVelichkoPI will update at some point to describe how the passthrough feature works.

Copy link
Contributor

@YuriyVelichkoPI YuriyVelichkoPI left a comment

Choose a reason for hiding this comment

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

LGTM

@bretg bretg merged commit 64e1738 into prebid:master Apr 4, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants