react-native-media-console icon indicating copy to clipboard operation
react-native-media-console copied to clipboard

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

Open vkukade-altir opened this issue 1 year ago • 4 comments

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?

vkukade-altir avatar Aug 22 '23 14:08 vkukade-altir

@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

LunatiqueCoder avatar Aug 22 '23 16:08 LunatiqueCoder

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

vkukade-altir avatar Aug 24 '23 13:08 vkukade-altir

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 avatar Aug 24 '23 14:08 LunatiqueCoder

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

LunatiqueCoder avatar Sep 27 '23 15:09 LunatiqueCoder