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: iOS constraints when controls are enabled and video is inside a ScrollView #4383

Conversation

efstathiosntonas
Copy link
Contributor

Fixes:

Summary

It seems that going from AVPlayerLayer to AVPlayerViewController when controls are enabled breaks the layout. By manually setting the constraints it keeps the size/position intact.

@efstathiosntonas
Copy link
Contributor Author

@freeboub I messed up the rebase/fork sync 😢 , wtf webstorm....

let me know if you want to ditch this PR and create a new clean one

@freeboub
Copy link
Collaborator

@freeboub I messed up the rebase/fork sync 😢 , wtf webstorm....

let me know if you want to ditch this PR and create a new clean one

I don't care, if you prefer to open a new one, it is ok for me !
Regarding to the control question, I think it will necessary be only if controls is true where you place the code .

@efstathiosntonas
Copy link
Contributor Author

@freeboub ok then, this is g2g. Thank you for looking into it.

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.

LGTM 👍

kotlin linter issue will be fixed here 6f703b3 - we just need to bump actions/upload-artifact version

I have created separate PR #4408, so you can do rebase (but you do have to, as you did not change any kotlin code)

@efstathiosntonas efstathiosntonas force-pushed the fix/ios_constraints_when_controls_enabled branch from 63d719e to 51ca930 Compare February 10, 2025 08:48
@efstathiosntonas
Copy link
Contributor Author

@KrzysztofMoch hi, just rebased, checks are fine, no kotlin checks fired 👍

@ChrisEdson
Copy link

Heya - any chance this can be merged? It's currently blocking my upgrade to RN Video 6 and consequently new React Native versions :)

@freeboub freeboub merged commit a8ca97f into TheWidlarzGroup:master Feb 15, 2025
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.

4 participants