search

dailymotion / dailymotion-sdk-js

Dailymotion JavaScript client API
https://developers.dailymotion.com/api/platform-api/
MIT License
34 stars 33 forks source link

Display the AdLog payload #89

Closed ZacharieTFR closed 3 years ago

ZacharieTFR commented 3 years ago

The player emit the ad_log event which forward the one triggered by VPAID. Some partner would like to access the adlog payload that can contain vpaid debug data.

The _dispatch method is getting really old, it was written 9 years ago (and supports IE 8/9 while the player doesn't), as we need this feature as soon as possible, the refacto will be done later.

johansatge commented 3 years ago

Naive question because I don't have full context, why doesn't ad_log have the same payload format than the other public player events?

rumesh commented 3 years ago

Naive question because I don't have full context, why doesn't ad_log have the same payload format than the other public player events?

Right now the payload is not sent with the event, ad_companions event set the payload as player state and can be access via player.companionAds which is not really a best practice. Event datas shouldn't be stored as state but only accessible at the event level. That's why we implemented it like this. Long term fix, we should use this pattern to send the payload of the event if no blocker? WDYT?

johansatge commented 3 years ago

Understood 👍

As discussed it sounds a bit weird indeed to have a player.adLogs property... Maybe we can go ahead with this solution but restrict it to the ad_log event? To limit the impact on the existing code and doc.

ZacharieTFR commented 3 years ago

Understood 👍

As discussed it sounds a bit weird indeed to have a player.adLogs property... Maybe we can go ahead with this solution but restrict it to the ad_log event? To limit the impact on the existing code and doc.

Done 👍 Thanks for the review !