[Video Module] Include a type property in all events #9771
Conversation
|
I re-ran the CircleCI job and we have all the checks passed now. |
|
Hi @pm-harshad-mane and @musikele this has been open for a while. Any blockers to merging ? |
|
just struggled with tasks at my regular work. Will check out now. |
| @@ -1,5 +1,19 @@ | |||
| import { videoKey } from '../constants/constants.js' | |||
|
|
|||
| export function getExternalVideoEventName(eventName) { | |||
| if (!eventName) { | |||
| return ''; | |||
| } | |||
| return videoKey + eventName.replace(/^./, eventName[0].toUpperCase()); | |||
There was a problem hiding this comment.
Here are we assuming that all video players have the event names have the prefix video ?
Should it be configurable with the default value video?
Refer: https://github.com/prebid/Prebid.js/blob/master/libraries/video/constants/constants.js#L1
There was a problem hiding this comment.
@musikele the library code has very limited functionality, looks good to me but I have one doubt (mentioned in the above comment)
@karimMourra please check the above comment.
There was a problem hiding this comment.
@pm-harshad-mane these events are part of the module's API. When video players surface events, they surface them using the following constants https://github.com/prebid/Prebid.js/blob/master/libraries/video/constants/events.js and we append the video portion. It keeps the char count lower.
This is already documented https://docs.prebid.org/prebid-video/video-module.html
|
@karimMourra do we want to update some documentation along with these code changes? |
|
@pm-harshad-mane the documentation is already up to date https://docs.prebid.org/prebid-video/video-module.html , see the Events section |
|
Thanks @karimMourra and @musikele, I will merge these changes then. |
Type of change
Description of change
The
typeproperty is missing from certain events fired by the video module. For consistency, we have added a util.