-
Notifications
You must be signed in to change notification settings - Fork 220
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
Add Vimeo tracking plugin (close #1156) #1212
Conversation
8e22643
to
94a0c52
Compare
videoTitle: await player.getVideoTitle(), | ||
videoWidth: await player.getVideoWidth(), | ||
videoHeight: await player.getVideoHeight(), | ||
// 'getVideoUrl' can fail due to privacy settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All Vimeo getters return a Promise, but it's very difficult to tell if they all can actually fail. The getters all internally run this method but it's hard to track it through to where a failure could occur. Anecdotally, the only method I've had fail is getVideoUrl
, but it might be worth wrapping more of these getters in the tryVimeoPromise
so we can soft fail on missing data for optional properties.
2603701
to
d94e865
Compare
BundleMonFiles added (6)
Total files change +95.15KB 0% Final result: ✅ View report in BundleMon website ➡️ |
3c3a83c
to
0cd4d94
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! Is there an example coming to the React app in snowplow-javascript-tracker-examples
? Would be great to demo this and inspect the events.
plugins/browser-plugin-vimeo-tracking/src/mediaPluginBinding.ts
Outdated
Show resolved
Hide resolved
plugins/browser-plugin-vimeo-tracking/src/mediaPluginBinding.ts
Outdated
Show resolved
Hide resolved
plugins/browser-plugin-vimeo-tracking/src/mediaPluginBinding.ts
Outdated
Show resolved
Hide resolved
0d534e5
to
6557d0b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, great work! 👍
I think we should wait for the implementation of aggregating seek start and end events in the media plugin to prevent the flood of events when the user scrubs the video.
6dd2a8a
to
7be7fae
Compare
40b6ee6
to
63cd3f0
Compare
63cd3f0
to
7cb772b
Compare
This PR adds a plugin to automatically track Vimeo videos, utilising the Snowplow Media plugin.