refactor: Update autoplay sample and how we report adsWillAutoplay an… #562
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d adsWillPlayMuted. Also deprecates the options ad(s)WillPlayMuted and ad(s)WillAutoPlay. Fixes #341.
…tAdsWillAutoplay.
…f it's required to run the samples then it should probably be in dependencies.
ypavlotsky
approved these changes
Mar 23, 2018
examples/autoplay/index.html
Outdated
| type="video/mp4" ></source> | ||
| height="360" playsinline> | ||
| <source src="//rmcdn.2mdn.net/Demo/vast_inspector/android.mp4" | ||
| type="video/mp4" ></source> |
bustbr
pushed a commit
to bustbr/videojs-ima
that referenced
this pull request
Apr 24, 2018
googleads#562) * refactor: Update autoplay sample and how we report adsWillAutoplay and adsWillPlayMuted. Also deprecates the options ad(s)WillPlayMuted and ad(s)WillAutoPlay. Fixes googleads#341. * Update README to indicate that setAdWillPlayMuted is deprecated. * Use getPlayerOptions() instead of vjsPlayer.options_ in controller getAdsWillAutoplay. * Add can-autoplay to release script for gh-pages. * Move can-autoplay from devDependencies to dependencies. I feel like if it's required to run the samples then it should probably be in dependencies. * Fix lint errors. * Move accidental extra file. * Fix example - check unmuted support first, then muted.
| @@ -91,7 +91,7 @@ the previous snippet. A summary of all settings follows: | |||
| | adLabel | string | Replaces the "Advertisement" text in the ad label. Added for multilingual UI support. | | |||
| | adsRenderingSettings | object | JSON object with ads rendering settings as defined in the IMA SDK,Docs(1). | | |||
| | autoPlayAdBreaks | boolean | Whether or not to automatically play VMAP or ad rules ad breaks. Defaults,to true. | | |||
| | adWillPlayMuted | boolean | Notifies the SDK whether the player intends to start ad while muted. Changing this setting will have no impact on ad playback. Defaults,to false. | | |||
| | **deprecated** adWillPlayMuted | boolean | Notifies the SDK whether the player intends to start ad while muted. Changing this setting will have no impact on ad playback. Defaults,to false. | | |||
There was a problem hiding this comment.
Not seeing this as being deprecated. Where do you see this as deprecated? https://developers.google.com/interactive-media-ads/docs/sdks/html5/v3/apis#ima.AdsRequest.setAdWillPlayMuted
There was a problem hiding this comment.
This isn't deprecated by IMA, we're just deprecating the setting in the plugin because we added logic to detect it automatically. We don't need folks who implement the plugin to tell us if their ad will play muted or not - we can just check if it will within the plugin.
There was a problem hiding this comment.
That makes sense. FYI, the auto-detect wasn't working with our implementation, we got a message from Google saying that we weren't setting this when we should be.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
…d adsWillPlayMuted.
Also deprecates the options ad(s)WillPlayMuted and ad(s)WillAutoPlay.
Fixes #341.
Fixes #484.