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

don't recreate the player for video changes #397

Open
wants to merge 1 commit into
base: canary
Choose a base branch
from

Conversation

iyzana
Copy link

@iyzana iyzana commented Jun 19, 2023

I am not sure why the player is being reset when the videoId changes. That case is already handled by shouldUpdateVideo as far as I can tell. I tested my change in Firefox and Google Chrome and switching the video still works as expected.

The check has been introduced in #284 but I did not find any reason for it there.

Without this change the player automatically leaves fullscreen on video changes which is undesirable for me.
Additionally to that, fullscreen can't then be resumed in Google Chrome, because the youtube player believes it is still fullscreened (iyzana/vsync#53).

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit ea68cdc:

Sandbox Source
react-youtube-example Configuration

@iyzana iyzana changed the title feat: don't recreate the player for video change feat: don't recreate the player for video changes Jun 19, 2023
@iyzana iyzana changed the title feat: don't recreate the player for video changes don't recreate the player for video changes Jun 19, 2023
@iyzana
Copy link
Author

iyzana commented Jun 20, 2023

In the codesandbox the change can be seen when clicking "Change video". Without this change the player fades to white for a bit, with this change only the video itself is updated which is quite a bit faster.

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.

1 participant