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 triggers unexpectedly with resizeMode="cover" on mount #123

Open
coofzilla opened this issue Aug 14, 2024 · 3 comments
Open
Labels
bug Something isn't working

Comments

@coofzilla
Copy link

Problem:

When resizeMode is set to "cover", the component initializes _isFullscreen to true, which triggers the onEnterFullscreen callback on mount, even if the video is not actually in fullscreen mode. This causes unintended side effects, especially when onEnterFullscreen contains functional logic.

I believe this is the culprit:

const [_isFullscreen, setIsFullscreen] = useState<boolean>(
  isFullscreen || resizeMode === 'cover' || false,
);

If resizeMode === 'cover', _isFullscreen is set to true, leading to this useEffect:

// VideoPlayer.tsx (AnimatedVideoPlayer)

  useEffect(() => {
    if (toggleResizeModeOnFullscreen) {
      setResizeMode(_isFullscreen ? ResizeMode.COVER : ResizeMode.CONTAIN);
    }

    if (mounted.current) {
      if (_isFullscreen) {
        typeof events.onEnterFullscreen === 'function' &&
          events.onEnterFullscreen();
      } else {
        typeof events.onExitFullscreen === 'function' &&
          events.onExitFullscreen();
      }
    }
    // eslint-disable-next-line react-hooks/exhaustive-deps
  }, [_isFullscreen, toggleResizeModeOnFullscreen]);

Since onEnterFullscreen is triggered by _isFullscreen, any logic in onEnterFullscreen is executed even though fullscreen wasn't explicitly intended.

Example:

<VideoPlayer
  resizeMode="cover"
  isFullscreen={false}  // Doesn't prevent the side effect
  onEnterFullscreen={() => {
    // This logic runs unintentionally on mount
  }}
/>

Expected Behavior

onEnterFullscreen should only trigger when fullscreen is explicitly activated by the user or another deliberate action, not just by setting resizeMode to "cover".

Suggested Fix

Revisit how _isFullscreen is initialized or decouple resizeMode="cover" from automatic fullscreen activation. An interim solution is to ensure that resizeMode doesn't implicitly trigger fullscreen logic.

@LunatiqueCoder
Copy link
Owner

@coofzilla Oh wooow people have been complaining about it but I haven't been able to reproduce this. Really nice find!

@coofzilla
Copy link
Author

Active contributors are eligible to receive a license for all JetBrains IDEs:
https://www.jetbrains.com/ides/

am I cool enough to get one of these? 🤣

@LunatiqueCoder
Copy link
Owner

You're so cool and you've been really helpful this year for us, but unfortunately the rule is that you have to contribute actively with code :( Your username needs to appear in the commit history 🚀

image

SOURCE: https://www.jetbrains.com/community/opensource/



So @coofzilla open some PRs and let's get going! 🚀

@LunatiqueCoder LunatiqueCoder added the bug Something isn't working label Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants