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

Component should only update if a player exists #13

Merged
merged 1 commit into from
Feb 24, 2023

Conversation

Jackdupkevin
Copy link
Contributor

@Jackdupkevin Jackdupkevin commented Aug 28, 2022

This PR will...

  • In shouldComponentUpdate() we are checking if the player exists, and will then update event listeners. Currently event listeners are being updated before a player exists and that causes errors.

Why is this Pull Request needed?

  • Currently the player is not usable with on<Event> listeners.

Are there any points in the code the reviewer needs to double check?

Are there any Pull Requests open in other repos which need to be merged with this?

  • No

Addresses Issue(s):

@jwplayer-robot
Copy link

Can one of the admins verify this patch?

@mdaoust-sidlee
Copy link

Interested to know when this PR will get published, thanks in advance.

@gsinovsky
Copy link

@imbcmdth could we please get this PR approved and merged? thanks!

@derweili
Copy link

From what I can see here, this also fixes #17

@imbcmdth @mamaddox
As you are the ones how have most commits on this repo so assuming you are admins, could you review and potentially merge this PR?

Copy link
Contributor

@mamaddox mamaddox left a comment

Choose a reason for hiding this comment

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

Sorry for the delay and great catch! We'll try to get this out next week. I'll ping you all when it gets in.

@mamaddox mamaddox merged commit fea7828 into jwplayer:main Feb 24, 2023
@bcole2433
Copy link

Will this be pushed to the yarn package soon?

@mamaddox
Copy link
Contributor

mamaddox commented Mar 2, 2023

Hey everyone, we've just published a new version on npm. Let me know if you have any issues.

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.

9 participants