MailOnline / videojs-vast-vpaid

video.js vast plugin
MIT License
296 stars 231 forks source link

Issues in the README.md ? #161

Open DrLightman opened 8 years ago

DrLightman commented 8 years ago

Or it's just me that I don't know videojs :)

(a) In the step 4 you write:

videojs.plugin('ads-setup', function (opts) {

But the videojs docs, it says to use a plugin like:

videojs.plugin('examplePlugin', ... function ...);

var video = videojs('cool-vid'); video.examplePlugin({ ... });

So I'm not supposed to use it like:

video.ads-setup

Because Javascript will throw an error, am I right? I should define the plugin as:

videojs.plugin('adsSetup', function (opts) {

Instead?

(b) In the step 4 again:

adsEnabled: !!options.adsEnabled

This throws an error in console:

ReferenceError: options is not defined adsEnabled: !!options.adsEnabled ___^

That's it for now, and sorry I'm new to videojs & videojs-vast :-P

brokenmass commented 8 years ago

(a) If you define like

videojs.plugin('ads-setup', function (opts) {

while it's true that video.ads-setup({...}) will throw an error you can obviously use video['ads-setup']({...)) or configure the plugin at video js initialisation like explained in the docs

 videojs('vidId', {
      plugins: {
        'ads-setup': {...}
      }
    });

(b) thats an error in our readme :tada: correct code is

adsEnabled: !!opts.adsEnabled

will fix it asap