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

Support Linear ads events and Player Operation events for VAST 4.1 #296

Conversation

unglud
Copy link

@unglud unglud commented Jul 11, 2019

Support Linear ads events and Player Operation events for VAST 4.1
moved mocha configs to separate file to be able run any isolated test from IDE

Description

VAST 4.1 contains additional tracking events: playerExpand/playerCollapse which eventually replaces the fullscreen/exitFullscreen events and loaded which track then everything loaded for playing.

Type

  • Breaking change
  • Enhancement
  • Fix
  • Documentation
  • Tooling

…ST 4.1

moved mocha configs to separate file to be able run any isolated test from IDE
@kobawan kobawan added this to the 3.0 milestone Jul 12, 2019
@kobawan kobawan added the 3.0 3.0 roadmap label Jul 12, 2019
Copy link
Contributor

@kobawan kobawan left a comment

Choose a reason for hiding this comment

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

LGTM! Just some minor comments about jsdocs

* or to the extent that it is ready to play the media
* Calls the load tracking URLs.
*
* @emits VASTTracker#complete
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't it emit 'loaded"?

@@ -278,6 +278,7 @@ export class VASTTracker extends EventEmitter {
setExpand(expanded) {
if (this.expanded !== expanded) {
this.track(expanded ? 'expand' : 'collapse');
this.track(expanded ? 'playerExpand' : 'playerCollapse');
Copy link
Contributor

Choose a reason for hiding this comment

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

please add jsdocs for these trackers as well :)

Copy link
Contributor

@eliamaino-fp eliamaino-fp left a comment

Choose a reason for hiding this comment

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

LGTM 👍 @kobawan green light on my side to merge this

@kobawan kobawan merged commit fd34e95 into dailymotion:3.0-version Jul 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.0 3.0 roadmap
Development

Successfully merging this pull request may close these issues.

None yet

3 participants