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

[AD-963] Add documentation for JW Player RTD Provider #2427

Merged
merged 7 commits into from
Oct 28, 2020

Conversation

karimMourra
Copy link
Contributor

Required for prebid/Prebid.js#5844

dev-docs/modules/jwplayerRtdProvider.md Outdated Show resolved Hide resolved
@karimMourra
Copy link
Contributor Author

@bretg I addressed your comments, could you please review ? Thanks

}
});
```

3) In order to prefetch targeting information for certain media, include the media IDs in the `jwplayerRtdProvider` var and set `waitForIt` to `true` before calling `setConfig`:
3) In order to prefetch targeting information for certain media, include the media IDs in the `jwplayer` var and set `waitForIt` to `true` before calling `setConfig`:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i disagree with this change, there is no jwplayer var. I think this would be confusing to the reader.
var jwplayer = {...}; would be a jwplayer var. I'll address this

Comment on lines 110 to 118
fpd: {
context: {
data: {
jwTargeting: {
segments: ['123', '456'],
content: {
id: 'jw_abc123'
}
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is wrong, we are not adding the targeting information to fpd, we are adding it on the bid object. We read from adUnit.fpd and write to bid.

@bretg bretg added LGTM and removed needs work labels Oct 28, 2020
@bretg bretg merged commit 89916ca into prebid:master Oct 28, 2020
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.

2 participants