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

include a renderer for a specific mediaType #2343

Merged
merged 5 commits into from
Nov 5, 2020

Conversation

MikeSperone
Copy link
Contributor

Documentation describing the new changes in a PR to address this issue:
prebid/Prebid.js#3231

A publisher can define a renderer for a specific mediaType - example, defining just a video renderer, so that a display can render with its own, or a separately defined renderer.

pull in updates from prebid
@bretg
Copy link
Contributor

bretg commented Oct 14, 2020

fixed conflicts

dev-docs/show-outstream-video-ads.md Show resolved Hide resolved
1. If a `renderer` is associated with the Prebid adUnit, it will be used to display any outstream demand associated with that adUnit. Below, we will provide an example of an adUnit with an associated `renderer`. If that renderer is specified as backup only, it will only be used when no other renderer is found.
2. If no `renderer` is specified on the Prebid adUnit, Prebid will invoke the renderer associated with the winning (or selected) demand partner video bid. Choosing a backup only renderer allows publishers to access demand with or without an attached renderer.
1. If a `renderer` is associated with the Prebid adUnit's video mediaType, it will be used to display any outstream demand associated with that adUnit with a mediaType of "video".
2. If a `renderer` is associated with the Prebid adUnit, it will be used to display any outstream demand associated with that adUnit. Below, we will provide an example of an adUnit with an associated `renderer`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would again remove the word outstream here, and perhaps indicate that the url would be invoked regardless of mediaType. Also, I think it makes sense to say this is legacy and #1 is now the preferred way.

@@ -67,13 +65,16 @@ To display an outstream video, two things are needed:

Prebid.js will select the `renderer` used to display the outstream video in the following way:

1. If a `renderer` is associated with the Prebid adUnit, it will be used to display any outstream demand associated with that adUnit. Below, we will provide an example of an adUnit with an associated `renderer`. If that renderer is specified as backup only, it will only be used when no other renderer is found.
2. If no `renderer` is specified on the Prebid adUnit, Prebid will invoke the renderer associated with the winning (or selected) demand partner video bid. Choosing a backup only renderer allows publishers to access demand with or without an attached renderer.
1. If a `renderer` is associated with the Prebid adUnit's video mediaType, it will be used to display any outstream demand associated with that adUnit with a mediaType of "video".
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would remove the word outstream here

Copy link
Contributor

Choose a reason for hiding this comment

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

@patmmccann - this document is called "Show Outstream Video Ads", so one argument is that this page should be focused on that use case.

Perhaps that implies we need a new page that focuses on "Multiformat Renderer" support?

code: 'video1',
// This renderer would work the same as it does above...
renderer: {
url: 'example.com/publishersCustomRenderer.js',
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think we should make this an example of the new "preferred" way and remove this altogether? what do you think

Copy link
Collaborator

Choose a reason for hiding this comment

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

also I would change line 171 to refer to your pull request, 5760

@bretg
Copy link
Contributor

bretg commented Nov 3, 2020

I think we should leave this doc outstream focused and create a new doc. Opened issue #2465 to track. The new doc may not be done in time for PBJS release tomorrow.

@patmmccann
Copy link
Collaborator

patmmccann commented Nov 3, 2020 via email

@MikeSperone
Copy link
Contributor Author

Sorry to leave this open for so long! I am able to make any changes today.

I agree on a separate page for multiFormat renderers, because both my change and the recent change to allow for a backup renderer can apply to all formats.

I can make the changes for this ticket today, and I can start work on #2465

To close this ticket, it looks like I should leave any "outstream" language, but still make the changes to

  • show that ex. number 1 is the preferred way
  • and refer to pull request 5760 in line 171

@patmmccann
Copy link
Collaborator

patmmccann commented Nov 4, 2020 via email

Documentation now states that applying the renderer property to the
mediaType property ('video' in this page) is preferred to applying it
on the adUnit level.  The example code also shows this.

Also refers to the pull request where this change was made.
@bretg bretg merged commit 54d3ed3 into prebid:master Nov 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants