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

Feature request/suggestion: conditional values for start and end #37

Open
codepreneur opened this issue Oct 28, 2016 · 3 comments
Open

Comments

@codepreneur
Copy link

codepreneur commented Oct 28, 2016

So this could be possible:

    player.overlay({
      content: 'Video has been paused',
      debug: false,
      overlays: [{
        content: `<h1>${title.substr(0, 60)}...</h1>`,
        showBackground: false,
        class: 'video-overlay',
        start: 'loadstart || pause',
        end: 'playing || vast.adStart',
      }]
    })

Or is it already possible ? Because without these conditionals its impossible to apply overlays during complex scenarios such as VPAID/interactive ads and I have to remove those overlays manually.

@gkatsev @misteroneill @dmlap

I am happy to do a PR if you guys give me a go ahead.

@misteroneill
Copy link
Member

Interesting idea! I say go for it.

Though, I do have one concern with a syntax like 'loadstart || pause' - it seems prone to breakage and it begs the question: how far does it go in supporting JS logic that it would essentially need to be evaled?

I think supporting an array of events would be better:

start: ['loadstart', 'pause'],
end: ['playing', 'vast.adStart']

That would trigger an overlay to start on the first of loadstart or pause. Then it would end on the first of playing or vast.adStart.

Moving beyond that - not necessarily for the same PR - we can expand on that further by allowing an object-based configuration, which supports a conditional function that determines whether or not the overlay is allowed to start. This is a bit of an unrealistic example, but...

start: {

  // Start at 1 second.
  at: 1,

  // Start on any of these events.
  on: ['loadstart', 'pause'],

  // Some kind of conditional function that determines if the overlay can start.
  condition: function(player, event) {
    return player.currentTime() < 10;
  }
}
  • start: {at: 1} is synonymous with start: 1
  • start: {on: 'foo'} is synonymous with start: 'foo'
  • start: {on: ['foo', 'bar']} is synonymous with start: ['foo', 'bar']
  • And condition is only meaningful in the presence of at or on.

That would open up a ton of customizable options and leave it mostly up to the implementer.

@gkatsev
Copy link
Member

gkatsev commented Nov 1, 2016

An array also matches videojs's support for multiple events.

@mister-ben
Copy link
Contributor

This would be useful. Conditions would also help with #31 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants