search

LunatiqueCoder / react-native-media-console

A React Native video player. Built with TypeScript ❤️
MIT License
184 stars 28 forks source link

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

Closed vkukade-altir closed 1 month ago

vkukade-altir commented 11 months ago

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 commented 11 months ago

@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: https://github.com/LunatiqueCoder/react-native-media-console/issues/49#issuecomment-1414058736

Let me know if you need any help

vkukade-altir commented 11 months ago

@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 commented 11 months ago

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 commented 9 months ago

@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.