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

onEnterFullscreen and onExitFullscreen both are also being executed on first render of video component. #76

Closed
vkukade-altir opened this issue Aug 22, 2023 · 4 comments · Fixed by #91

Comments

@vkukade-altir
Copy link

Hi @LunatiqueCoder , issue I noticed is -

Expected - onEnterFullscreen and onExitFullscreen both should be executed only when user clicks on "Full screen ICON" on bottom right corner.
Issue - In addition to "Full screen ICON" click , onEnterFullscreen and onExitFullscreen both are also being executed on first render of this component.

I don't know if this is the issue or is it correct ? May be am I missing anything ?

Also this may seem as a dumb question. I wanted to go over code and create PR for this issue. But I forked the project, I don't see Android and IOS folders to get started with. How can I run this on the react native android/ios app and start debugging?

@LunatiqueCoder
Copy link
Owner

@vkukade-altir Nice catch! Didn't test it, but looking at the code, it sure happens.

I wrote some kind of a contributors guideline here: #49 (comment)

Let me know if you need any help

@vkukade-altir
Copy link
Author

@LunatiqueCoder I checked the code and I see that depending on the value of isFullscreen or resizeMode === 'cover'
its calling onEnterFullscreen and onExitFullscreen on first mount also. In which scenarios its necessary to call this functions on first mount?

I have one question - Should we disable the behaviour of executing onEnterFullscreen and onExitFullscreen on first mount of component and keep the behaviour of calling onEnterFullscreen and onExitFullscreen only on click of "Full screen ICON". What do you say?

But if we do this would we miss this next scenario ? - where user wants to enter full screen mode by default when video mounts.

@LunatiqueCoder
Copy link
Owner

That's a good question worth investigating. Unfortunately, I have more critical issues to look into, so I'm really not sure if I'll find the time in the next 1-2 weeks to look into it. :(

@LunatiqueCoder
Copy link
Owner

@vkukade-altir From what I tested, only one of those methods are being executed on the first render, based on the isFullscreen prop (default false).

if isFullscreen === true => onEnterFullscreen()
else
onExitFullscreen()

Here is the full source code: https://github.com/LunatiqueCoder/react-native-media-console/blob/master/packages/media-console/src/VideoPlayer.tsx#L299C1-L312C53

Not sure how to handle this in an elegant way, but will keep investigating.

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 a pull request may close this issue.

2 participants