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

PBjs Core (Native): ortb tracker support #8905

Merged

Conversation

musikele
Copy link
Contributor

Type of change

  • [ x ] Feature

Description of change

Native ORTB spec allows clicktrackers to be defined on individual assets; this PR adds this feature, together with PUC #150.

Other information

Accompaining with prebid/prebid-universal-creative#150

@ChrisHuie ChrisHuie changed the title Native Ortb - Trackers support PBjs Core (Native): ortb tracker support Aug 29, 2022
Copy link
Collaborator

@dgirardi dgirardi left a comment

Choose a reason for hiding this comment

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

LGTM with a minor styling suggestion; also noting that this must go out before the PUC changes I believe.

src/native.js Outdated
@@ -305,8 +305,27 @@ export function fireImpressionTrackers(nativeResponse, {runMarkup = (mkup) => in
}
}

export function fireClickTrackers(nativeResponse, {fetchURL = triggerPixel} = {}) {
(nativeResponse.link?.clicktrackers || []).forEach(url => fetchURL(url));
export function fireClickTrackers(nativeResponse, {fetchURL = triggerPixel, assetId} = {}) {
Copy link
Collaborator

@dgirardi dgirardi Aug 29, 2022

Choose a reason for hiding this comment

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

it's just style, but I'd prefer to either merge params together or keep dependencies separate from inputs (either fireClickTrackers({nativeResponse, assetId, {...dependencies}}) or fireClickTrackers(nativeResponse, assetId, {...dependencies}) look better to me).

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 prefer the first one.

@patmmccann patmmccann merged commit 806c4d9 into prebid:master Aug 31, 2022
JacobKlein26 pushed a commit to nextmillenniummedia/Prebid.js that referenced this pull request Feb 9, 2023
* handle clicktrackers the ORTB way

* support for clicktrackers on assets

* parameters are now explicitly defined

Co-authored-by: Michele Nasti <michele@rtk.io>
jorgeluisrocha pushed a commit to jwplayer/Prebid.js that referenced this pull request May 23, 2023
* handle clicktrackers the ORTB way

* support for clicktrackers on assets

* parameters are now explicitly defined

Co-authored-by: Michele Nasti <michele@rtk.io>
@robertrmartinez robertrmartinez deleted the native_ortb_trackers_support branch July 5, 2023 19:53
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.

None yet

5 participants