-
-
Notifications
You must be signed in to change notification settings - Fork 530
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
Incorrect layout when navigating between screens of different orientation on iOS #1738
Comments
Hi @thomas-coldwell, thanks for reporting. Also, do you see message from UIKit as in #1732? |
I'm downloading iOS 15.5 to test out on the simulator now and will report if the issue exists there. I can confirm I do see the UIKit |
Thank you! |
I can confirm this issue does not occur on iOS 15.5 (tested on an iPhone 13 Pro Max & iPhone 8 simulator just to make sure it wasn't any weird screen size issue). I'll also test with the |
I've just tested it on iOS 16 (iPhone 14 simulator) again with the changes for the |
Also just another bit of info to add to this that might help us solve it - |
Thanks for the info and the reproduction repo Thomas! I ended up spending a bit of time trying to debug this, trying to follow which code paths calls what. For my case, I ended up with a temporary workaround, in which case I was not able to reproduce the issue in your reproduction repo. I added the following line to RNSScreenStack.mm, in your reproduction repo
Since this was enough to solve our use case by doing a patch-package, I didn't debug any further as of now. |
That works really well @vinkim! I think I tried this before (outside the sandbox project) and it didn't perform the layout, but will re-test with this! Thanks for helping to debug it 🙌 |
I am experiencing a very similar issue, though slightly different I think. This all started after switching to the NativeStackView from StackView. Screens render well in either orientation, but on one screen if you rotate, it is not updated to the new layout. I have tired manually setting the height and width based on the device window dimensions, but that does not make a change. I also tried the patch suggested by @vinkim which also did not solve the issue. Trying to make a small repro case but don't have the time just yet. Currently testing on: |
I have the same issue, Is someone fixed it? |
Thank you! Works for me :) |
I am facing a similar issue on iOS 16.7.1. My setup is slightly more complicated, I control orientation manually with <Stack.Screen
name="landscape-full-screen"
component={LandscapeFullScreen}
options={{
animation: 'none',
// ... Packages:
|
Hey @thomas-coldwell, @vinkim (huge shout out to you for suggesting the change!), @Thenlie, @Salmankhan033, @feedthedevil, @madox2! Sorry for the late response in this issue 😅 I've created the PR with the suggested by @vinkim changes. I've also came to another solution that you can find in the PR, but I believe You can check those changes by changing line with "react-native-screens": "software-mansion/react-native-screens#@tboba/fix-updating-bounds" |
As I've read in #1738 that this issue is also reproducible on the old architecture (but it's hard to reproduce it there) I'm reverting this change just in case if this problem could also occur there.
any idea when this fix will get merged and released ? |
Hello! I'm also interesting when it'll be fixed. But I still can see this ruge UI transformations when layout transforms from landscape to portrait and vice versa. It looks like layout transforms first and only after it apply new width and height (and app layout looks broken for a part of second in a middle). |
…1970) ## Description Currently while trying to change the interface orientation there's a bug that doesn't layout the next screen, causing the old frame to remain (which means that the next screen will be rotated regarding to the previous orientation. This PR fixes that problem by calling `[_controller.view setNeedsLayout]` on the `navigationController`, where the view is being rotated. When I was testing this PR I also came to another solution of setting the frame in `viewWillTransitionToSize` but I didn't spot any differences between one solution and this one. ```objc - (void)viewWillTransitionToSize:(CGSize)size withTransitionCoordinator:(id<UIViewControllerTransitionCoordinator>)coordinator { [super viewWillTransitionToSize:size withTransitionCoordinator:coordinator]; #ifdef RCT_NEW_ARCH_ENABLED [coordinator animateAlongsideTransition:^(id<UIViewControllerTransitionCoordinatorContext> coordinator) { self.screenView.frame = CGRectMake(0, 0, size.width, size.height); } completion:^(id<UIViewControllerTransitionCoordinatorContext> coordinator) { [self.screenView setNeedsLayout]; }]; #endif } ``` I've also tried to make a workaround of a reaaaally old bug in React Native: facebook/react-native#16060, but unfortunately I failed with that 😕 Closes #1738. ## Changes - Adds call to `setNeedsLayout` during the interface orientation ## Screenshots / GIFs ### Before https://github.com/software-mansion/react-native-screens/assets/23281839/bcc366ab-9757-4a31-89e0-241c22cdd5c6 ### After https://github.com/software-mansion/react-native-screens/assets/23281839/6634379a-2697-46a3-aed6-3cb156616817 ## Test code and steps to reproduce You can find in this PR `Test1970.tsx` that contains test test I've tested while making this change. ## Checklist - [X] Included code example that can be used to test this change - [x] Ensured that CI passes
Unfortunately, I'm still experiencing the same issue on the latest v3.31.1 - the weird transition between portrait and landscape and vice versa on iOS. At same time it works smoothly on Android. I'm using
I'm trying to make it work in two ways:
I tried to add |
…oftware-mansion#1970) ## Description Currently while trying to change the interface orientation there's a bug that doesn't layout the next screen, causing the old frame to remain (which means that the next screen will be rotated regarding to the previous orientation. This PR fixes that problem by calling `[_controller.view setNeedsLayout]` on the `navigationController`, where the view is being rotated. When I was testing this PR I also came to another solution of setting the frame in `viewWillTransitionToSize` but I didn't spot any differences between one solution and this one. ```objc - (void)viewWillTransitionToSize:(CGSize)size withTransitionCoordinator:(id<UIViewControllerTransitionCoordinator>)coordinator { [super viewWillTransitionToSize:size withTransitionCoordinator:coordinator]; #ifdef RCT_NEW_ARCH_ENABLED [coordinator animateAlongsideTransition:^(id<UIViewControllerTransitionCoordinatorContext> coordinator) { self.screenView.frame = CGRectMake(0, 0, size.width, size.height); } completion:^(id<UIViewControllerTransitionCoordinatorContext> coordinator) { [self.screenView setNeedsLayout]; }]; #endif } ``` I've also tried to make a workaround of a reaaaally old bug in React Native: facebook/react-native#16060, but unfortunately I failed with that 😕 Closes software-mansion#1738. ## Changes - Adds call to `setNeedsLayout` during the interface orientation ## Screenshots / GIFs ### Before https://github.com/software-mansion/react-native-screens/assets/23281839/bcc366ab-9757-4a31-89e0-241c22cdd5c6 ### After https://github.com/software-mansion/react-native-screens/assets/23281839/6634379a-2697-46a3-aed6-3cb156616817 ## Test code and steps to reproduce You can find in this PR `Test1970.tsx` that contains test test I've tested while making this change. ## Checklist - [X] Included code example that can be used to test this change - [x] Ensured that CI passes
Description
This issue is hard to consistently reproduce, however, when navigating between screens with a different orientation e.g. Landscape -> Portrait, the new screen is incorrectly laid out with the previous screen's orientation - see screenshot below of the view hierarchy after pushing a new portrait screen.
Simulator.Screen.Recording.-.iPhone.14.-.2023-03-14.at.18.29.56.mp4
As you can see the
RNSScreenView
(child of theRNSScreen
UIViewController) that I've navigated to has had its frame set to landscape dimensions even though it is actually a portrait screen, resulting in a broken layout.I've added a log to the
setFrame
method of theRNSScreenView
to see where this gets called from and with what dimensions (if you scroll to the right you can see the calling method):There are a couple of interesting things to note in the logs above:
[UINavigationController _startCustomTransition:]
& related transition animation methods set the new screen to landscape dimensions (even though the screen is specified as portrait). It looks like this is due to the fact the UINavigationController'ssupportedInterfaceOrientations
are specified by its top view controller which in this case would still be landscape (before the transition has started and before the new view controllers have been set) & then the UINavigationController internally sets the final frame for the view we are transitioning to based on this in its various animated transition methods.[UINavigationController _layoutViewController:]
presumably from it some other update to the navigation controller triggering a re-layout with the correct, portrait dimensions for the screen.Expected Behaviour
That when navigating to a portrait screen it should correctly layout with portrait dimensions.
Steps to reproduce
setFrame
in theRNSScreenView
Snack or a link to a repository
https://github.com/thomas-coldwell/rnscreen-orientation-glitch
Screens version
3.20.0
React Native version
0.71.3
Platforms
iOS
JavaScript runtime
Hermes
Workflow
React Native (without Expo)
Architecture
Paper (Old Architecture)
Build type
Debug mode
Device
iOS simulator
Device model
iPhone 14
Acknowledgements
Yes
The text was updated successfully, but these errors were encountered: