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

Add poster to the options the tech are receiving #2338

Closed
wants to merge 2 commits into from
Closed

Add poster to the options the tech are receiving #2338

wants to merge 2 commits into from

Conversation

eXon
Copy link
Contributor

@eXon eXon commented Jul 10, 2015

The only way I can know if I must set the default YouTube poster is by checking if the option poster is already set. However, that information is not sent to the tech right now.

The only way I can know if I must set the default YouTube poster is by checking if the option poster is already set. However, that information is not sent to the tech right now.
@heff
Copy link
Member

heff commented Jul 10, 2015

Makes sense to me. Does the video.js poster not cover up the youtube poster?

@eXon
Copy link
Contributor Author

eXon commented Jul 10, 2015

The tech needs to set the YouTube poster url. If we use the one displayed by the iframe, we have a big red ugly play button (really weird with the vjs poster). When we set a VJS poster, it hides the whole iframe until you click play.

@@ -502,7 +502,8 @@ class Player extends Component {
'autoplay': this.options_.autoplay,
'preload': this.options_.preload,
'loop': this.options_.loop,
'muted': this.options_.muted
'muted': this.options_.muted,
'poster': this.options_.poster
Copy link
Member

Choose a reason for hiding this comment

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

@eXon could you change this to use this.poster()? That would make sure that if the poster changes, later techs will get the new poster.

@heff heff closed this in dd0752e Jul 24, 2015
@heff
Copy link
Member

heff commented Jul 24, 2015

Merged in, thanks! I think we still have a little to figure out around the poster in #2339.

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

Successfully merging this pull request may close these issues.

2 participants