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.

Sign up for GitHub

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

Jump to bottom

Prevents showing thumbnail and the video embed after changing the video ID #48

Closed
wants to merge 1 commit into from
Closed

Prevents showing thumbnail and the video embed after changing the video ID #48

wants to merge 1 commit into from

Conversation

ronnyroeller
Copy link

The player is initialized when playing the video in the component for the first time. The fix explicitly hides the player when changing the video ID afterwards, and brings the player back once the user clicks again the thumbnail.

Fixes #46

@ronnyroeller ronnyroeller mentioned this pull request Nov 17, 2015
Open
@jeffposnick jeffposnick self-assigned this Dec 2, 2015
@jeffposnick
Copy link
Contributor

Thanks for contributing! However, the code in this PR will lead to the YouTube embed being hidden whenever there's a change to the underlying videoId property, regardless of whether there's a custom thumbnail or not. This leads to a big empty space instead of the new video's standard thumbnail being displayed when thumbnail is not set:

image

I understand what you're trying to accomplish here, but I'm not sure that this is the right approach. If you'd like to modify your PR, I think a more resilient approach would be to fiddle with the thumbnail's layering (perhaps via z-index) and ensure that your solution works when thumbnail isn't set at all. If you'd rather not make additional changes to this PR, I can try to fix #46 in an alternative PR.

@ronnyroeller
Copy link
Author

Thanks for the feedback @jeffposnick . I think I prefer to close the PR and wait for your PR for #46.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jeffposnick jeffposnick

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Changing the video shows the thumbnail and the video embed
2 participants
@ronnyroeller @jeffposnick