-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
fullscreen mode on IOS breaks layout when video ends #1319
Comments
@cobarx ping :) Hi. This bug has been tested on Ipad air 2 and Iphone 8. here's multiple screens that tries to explain how to reproduce it (taken from Iphone 8 IOS 12.1): on click we show the video, and native controls set to true we go to fullscreen : Then 2 bugs are happening here (on both Ipad Air & Iphone 8) when we go out of fullscreen : You can see now that the layout is broken, means that it's now take half of the screen. The upper side is scrollable tho. And if we stay on fullscreen until the video ends, on Ipad Air only screen freeze. We never go back to the app and need to restart it to take control again. QuestionsIs someone managed to reproduce this bug ? I tried to reproduce it with a snack.expo.io but it seems that react-native-video does not work with snack : https://snack.expo.io/@jonathan01/iphone-fullscreen |
@cobarx ping What do you think about this issue ? Did you ever seen it ? |
Still happens. Tested on iPad. After a video goes full screen and back, the view is not resized anymore when the tablet orientation change. |
@cristiangu So its reproducable, that's a good thing. Thanks for your feedback on this. |
@cristiangu Do you have a little basic white project that we can launch that reproduce this error ? With this it will be easier for maintainers like @cobarx to fix it. |
same here, layout is broken after the video goes full screen and back on IOS device |
I found that if i pressing the full screen button in the control bar, although the layout is broken, the full screen is take the whole screen that what i need. If i run this code |
@cobarx Hi, is this issue followed by anyone ? Since 2 month now and no maintainers take this issue. |
I'm having similar issues. When controls={true} fullscreen layout is broken. Everything is fine however when the controls prop is unspecified. |
@jonathangreco do you have a git repo with your code for testing? |
Hi sorry for not responding sooner. It is hard to keep up with the volume of issues that this project generates. I don't get to put in much time on this project for my work, so it's mostly in my free time and there isn't much activity from other maintainers to help with the load. I end up putting most of my time into reviewing PRs. However, since this is a crash it needs to get fixed asap. I should be able to take a look at this this week and see if I can sort it out. If you happen to be able to put together a PR that would be great, but I should be able to tackle it. |
@ashnfb Hi, I've not any git repo for testing at the moment. But maybe this snack https://snack.expo.io/@jonathan01/iphone-fullscreen can be mounted as a project on IOS to test it. (Snack not compile react-native-video, i can't make it work) @cobarx Don't be sorry, no problem, you may notice that I'm a patient guy,I completely understand how it's complicated to handle a community repository in addition of your work, I'm glad to see you in this thread now :) I'm not an objective C nor a Java developper sorry so I can't make a PR in order to fix this. It seems that it's easy to reproduce, because a lot of people came on this thread to confirm this is still happening. I hope you'll be able to reproduce easily. |
@cobarx @jonathangreco if you can wait until early next week, I will try and put together a PR for this Monday or Tuesday |
That's fine I won't have time to work on anything before next week. I'll
look forward to your PR when it's ready. Thanks for tackling this!
…On Wed, Jan 16, 2019, 8:58 AM Ash Mishra ***@***.***> wrote:
@cobarx <https://github.com/cobarx> @jonathangreco
<https://github.com/jonathangreco> if you can wait until early next week,
I will try and put together a PR for this Monday or Tuesday
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1319 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AD4AF4Hdzhxx2FtWCZdpWyvLGtfy3CJ2ks5vD1oygaJpZM4YWa34>
.
|
Example code is at this branch: Confirmed there are issues when controls={true}. Doesn't happen if controls are set to false. |
Then it's confirmed, happy here :) |
@jonathangreco I'm working on it. It looks like react-native is not getting onLayout (rotation changes) when in full-screen mode via the built-in controls, so when exiting it messes up the layout. Might need a hack to fix. |
@jonathangreco @cobarx fixed I'm not very happy with the fix; but it's the best I could come up with. Here's the diagnosis and solution: Normally, whenever we rotate the device, react-native components receive an "onLayout" event. In this code, there are 2 different methods for rendering video: AVPlayerLayer (doesn't have controls), and an embedded AVPlayerViewController (when controls are on). The problem is when AVPlayerViewController is in fullscreen and a rotation occurs, the "onLayout" is never sent to react-native's shadowViews. The solution is to force the reactViewController.view (the rootViewController) to update it's bounds to the rotated screen's bounds and mark the view as needing layout. This causes the right sequence of events -- ie. react-native calls onLayout correctly and layouts out all other subviews. Couple of other notes. The basic example, isn't very basic. I've added a new example which is really barebones, and necessary for testing bugs. Feel free to rename it in the PR as appropriate. |
whoaaa really thanks for the fix. |
@ashnfb Hi is this planned to be merged soon ? |
Hi @cobarx this can be merged
…________________________________
From: Jonathan <[email protected]>
Sent: February 15, 2019 1:43:23 AM
To: react-native-community/react-native-video
Cc: Mishra Ash; Mention
Subject: Re: [react-native-community/react-native-video] fullscreen mode on IOS crash layout when video ends (#1319)
@ashnfb<https://github.com/ashnfb> Hi is this planned to be merged soon ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1319 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AbpNLu8r80GBVpruMhpvYNhBavu4spbyks5vNoE7gaJpZM4YWa34>.
|
Hi @cobarx what do you need on this issue in order to merge this ? It would be nice if I can get this for my deployment of my app on the apple store next week :) Pleeeease :) |
I'm experiencing the same issue. Looking forward to the fix. |
@cobarx another ping from here, please do your best on this. |
I'm not having the best of luck reproducing this, but I trust @ashnfb's judgment on this and it sounds like the fix works for you @jonathangreco so I went ahead and merged it. I did find one issue with the fix. If I go fullscreen using the button on the onscreen controls, everything works fantastic. However if I use @ashnfb would you mind looking and seeing if you can diagnose this? |
When I test going fullscreen via the method on 4.4.0 it doesn't produce the red screen, however the controls are the wrong size and in the wrong position. |
Hi all, I'm sorry for the delay. I'm on parental leave for a newborn, but I will try and have a look at the issue Hampton found.
@phil, and others - the code has been merged into master, so you can make a direct reference to that commit (or later) in your packages for now.
…________________________________
From: Phil Gebauer <[email protected]>
Sent: March 20, 2019 10:54:30 PM
To: react-native-community/react-native-video
Cc: Mishra Ash; Mention
Subject: Re: [react-native-community/react-native-video] fullscreen mode on IOS crash layout when video ends (#1319)
@cobarx<https://github.com/cobarx> @ashnfb<https://github.com/ashnfb> what's the status on this? When is it going to be released? Thanks!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<#1319 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AbpNLoaOc25iDFGzBfdKS2tCbdn3n1X3ks5vYx6WgaJpZM4YWa34>.
|
Hi @jonathangreco, I think the problem is the scrollview needs a different set of styling to resize correctly in a parent view. The way I discovered the issue was giving all the views a different background color, and I was able to see the scrollview wasn't resizing. I haven't explored a solution, but I'll try and have a look at it this week. |
@ashnfb Hope you can do it, really thank you for your work, I will be able to test anything you'll push on my test repo |
@jonathangreco just dropping an update. I haven't been able to fix this with a scrollview yet. Normally onLayout is called when a view has a flex=1. This continues to work with Views and fullscreen, however, when a ScrollView is involved, exiting Fullscreen stops the flex=1 from triggering an onLayout change, which results in the layout being incorrect. It seems to be a rendering issue deeper in the React code; and I'm not sure there is a easy workaround. Maybe @cobarx can help us? |
I don't know if @cobarx can help. I'm only able to test your work on this one. I'm not a native developper yet, but a simple react-native user :) I can jus hope that this isssue will find an happy ending, since it's been a 8 month now. My app does not look the same without fullscreen. It's missing the "Wahouuu" effect ;p Again many thanks to you for the time passed on this. |
I don't know if it helps you, but I am using https://www.npmjs.com/package/react-native-orientation-locker for that. When the component is unmounted or when navigation occurs, I set the orientation lock back to the value I need. |
I am also experiencing this issue where the layout does not change back to portrait after rotating it to landscape. However, I noticed it does NOT happen on iPhone X (and any other screens with a notch) but, on the iPhone 6/7/8 it experiences this. Could this be a device related issue? |
Do you have an idea about the fix to this issue? @cobarx |
@jonathangreco I now have this working correctly! @CHaNGeTe was correct, and this is an orientation problem we're dealing at the react-native level, when re-rendering the layout of views. There are no code changes to react-native-video, it simply needs re-rendering on the js side - which you can do with a package, or listen to a change in Dimensions. I've attached an example for you here p.s. Make sure to use my fork to test. https://github.com/nfb-onf/react-native-video |
@ashnfb I tested it on an Iphone XR, the content is now centered when back to the app (exit fullscreen) but there still a white side...unusable. The rendering do its job, centered the content again. But the app use the half of the screen after rotating. So there is progress but it's still unusable :p I paste below my code of the screen, I also tested if your snippet, same issue. I did a yarn upgrade to pull your last commit from your repo, but no luck.
|
@jonathangreco I was also getting the 1/2 white; it's a bit tricky because scrollview is quite broken. Send me your project by email at [email protected] |
From emails between @ashnfb and myself it appears that React-navigation can cause the 1/2 white screen. But remove it (since it's a major components) does not seems to be a good idea, a package wich is incompatible with another is bad and have to be fixed from one side or the other. But even if the white screen can be fixed for now by removing the navigation system, we still have the issue when the video finishes (the root view is destroyed ?) So far, I don't think this workaround is fully stable, it needs more investigation and work around the component ScrollView. Maybe we can post an issue on react-native main package for the scroll view component ? @cobarx since you have a lot to catch up on this issue, I understand you don't want to involve, but we are really stuck on this at the moment, even if @ashnfb did a really great job to investigate on this, your help would be a great thing. We are near the solution, I'm sure we are |
In case this helps anyone: I have tried https://github.com/abbasfreestyle/react-native-af-video-player as well as Expo's video module (https://docs.expo.io/versions/latest/sdk/video/) and they both have the exact same issue. Some debugging has lead me to I ended up implementing a custom controls interface on top of Expo's video module so that I can prevent fullscreen mode. This turned out to be a much bigger undertaking than I had hoped for but at least it's a somewhat acceptable UX. I started with https://github.com/ihmpavel/expo-video-player for the controls interface but ended up copying and heavily modifying its code to get it to an acceptable level of UX as it's rather, well, quirky out of the box, and has some bugs and outdated dependencies. If anyone is interested I'd be happy to share my modified version. |
I discovered a new thing about this issue, this can maybe give a boost to its resolve. |
@fubar Hi, I didn't take the time to answer your post, what you did is pretty amazing, if I understand it correctly you have made native controls on iOS that not show the fullscreen button ? If so can you make a PR in this repo to allow via a props to remove the fullscreen button ? |
Hey folks, arrived on this issue after trying to troubleshoot an issue on my app on which just displaying video controls when not fullscreen would break the content size of scroll views when the react native view controller is not the whole screen size. Shouldn't the logic that observes the There's an if on the code over there that checks wether its full screen or not, but it only logs it. Applying this logic only on the fullscreen solves the problem I'm having on my App, but I couldn't verify if it still solves the other cases the workaround was intended to solve. Cheers o/ |
@marlonandrade I stumbled on this issue and your PR #1931 This seems to still break my layout (uses tab navigation) when going into fullscreen then back. Would it make sense to add some logic to the "not fullscreen" part to return the view back to its original size? Don't know Obj C that well to know what it'd look like in code, unfortunately. |
Hi @hedgerh, indeed there's no code path that restores the original size of the original react view and this will be a problem on my App as well. I'm not very familiar with the whole view hierarchy and its lifecycle, so dunno if this code path will get executed when getting back from fullscreen to be able to restore it. If u wanna give it a try, a quick hack to check it would be to save the original Unfortunately I don't have time right now to dedicate to that =( |
Fyi if you need a hack, I ended up getting things to work by removing the reactViewController frame resize, then insstalling
If I get a chance I may mess with the actual frame resize, but this may have to do. |
any update on this? |
A little late here but just in case anybody hits a similar issue as me.. I had a similar issue because I was setting a wrapping view style with I know this doesn't fit all cases (even for us) but hopefully this saves somebody from spending an hour debugging :) My component tree looked like: // assume that we don't have control of width of view that renders
// a MyVideoPlayer component
const MyVideoPlayer = (props) => {
return (
<View style={styles.container}>
<Video style={styles.videoPlayer} {...props} />
</View>
)
}
const styles = EStyleSheet.create({
container: {
height: 200,
width: '100%' // this breaks!
},
videoPlayer: { flex: 1 }
}) The fix: const styles = EStyleSheet.create({
container: {
height: 200,
width: Dimensions.get('window').width
},
videoPlayer: { flex: 1 }
}) |
Any ideas how to fix it when full screen video plays on React Native WebView ? Same issue as this library. I wonder if a RN bug. |
Hi everyone |
Current behavior
On IOS with the native players buttons I read a video and go to fullscreen.
When the video finishes, i go back to my app and then I switch from portrait to landscape (or vice versa)
Reproduction steps
Then your app not taking the whole screen.
Expected behavior
The app should take the whole screen.
Platform
Which player are you experiencing the problem on:
The text was updated successfully, but these errors were encountered: