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

fix: _paused is updated when video playback pause #4320

Merged

Conversation

giodevcoding
Copy link
Contributor

@giodevcoding giodevcoding commented Dec 4, 2024

Summary

Keep paused state in sync with playing state.

Motivation

I found that the video resumes from being paused automatically when foregrounding the app, which is an unexpected behavior. I wanted the playback state from the background, whether playing or paused, to persist from background to foreground to create a better experience for the users of my app.

Also a fix for #4321

Changes

Update _paused on RCTVideo alongside _isPlaying when the video playback (or time control status) changes.

I found that this was behavior was due to _paused on RCTVideo not being updated when the video is paused with native controls, as it is only updated by the setPause() method. The consequences of this are that setPause() is called within applyModifiers() which is called within applicationWillEnterForeground(). This call uses the current value of _paused (setPause(_paused)), which is false, and setPause() will play the video if the value given to it is false.

Updating _paused alongside _isPlaying keeps it update with the state of the player and fixes the issue.

Test plan

  1. Open Video
  2. Pause Video
  3. Background App
  4. Foreground App
  5. Video should remain paused (or whatever it's background state was when backgrounded)

Copy link
Member

@KrzysztofMoch KrzysztofMoch left a comment

Choose a reason for hiding this comment

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

Good catch!

@KrzysztofMoch KrzysztofMoch merged commit 3da4f1c into TheWidlarzGroup:master Dec 10, 2024
6 checks passed
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.

2 participants