-
-
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
LargeHeader stays small after pop/goBack/swipe gesture on iOS 14+ #649
Comments
This looks like it has nothing to do with SDK 39, its an issue with iOS 14. It works on iOS 13. The same code on iOS 13 will grow the large header back to its original size. |
Yeah, I can spot problems with this. It looks like some kind of bug because it doesn't happen in the |
@WoLewicki you are right, works great everywhere else in the system. |
The same here 👍 |
Does this behavior appear only while using |
For me, it happens when using |
Maybe this is related: iOS 14 Large title collapse when app run firstly. I experience this issue too, when I start the app or scroll to the top, navigate and go back on iOS 14. |
Can you check if #670 fixes the issue and does not introduce any new issues? |
I’ve got the same problem on my Navigation router (@WoLewicki your fix didn’t work for me). I’ve worked out a fix but we need to tweak the Here’s the fixed @implementation RCTScrollContentView
- (void)didMoveToWindow
{
[super didMoveToWindow];
RCTScrollView *scrollView = (RCTScrollView *)self.superview.superview;
[scrollView updateContentOffsetIfNeeded];
}
- (void)reactSetFrame:(CGRect)frame
{
[super reactSetFrame:frame];
RCTScrollView *scrollView = (RCTScrollView *)self.superview.superview;
if (!scrollView || !self.window) {
return;
}
RCTAssert([scrollView isKindOfClass:[RCTScrollView class]], @"Unexpected view hierarchy of RCTScrollView component.");
[scrollView updateContentOffsetIfNeeded];
}
@end |
Looks like it works @grahammendick 🎉. Are you going to submit a PR with this in the |
@WoLewicki I'm glad it works for you too! |
I agree with @grahammendick on this. If you're an unknown name in the original I think, you guys from SWM have a better reputation and are more likely to have success submitting something there. Is there a chance to fix this trough react-native-screens instead of fixing it upstream? @WoLewicki |
I tried fixing it upstream in my Navigation router but had no luck. The problem is we need to stop the |
@hirbod You're right, but the react-native repo is just one example of the problems you have if you're an unknown name in React. I've found it impossible to get my work looked at. If you're not a name then you're not getting in |
@WoLewicki can someone from your team create PR to react-native repository? |
It would need more debugging, submitting a PR with changes to a component used by almost every developer must have a very strong explanation and must ensure that no right behavior will be broken after it. |
For me, this quick hack worked. return(
<FlatList
contentOffset={{ x: 0, y: -1}}
contentInsetAdjustmentBehavior={"always"}
maintainVisibleContentPosition={
{
autoscrollToTopThreshold: -1,
minIndexForVisible: -1
}
}
>
{/*your content goes here */}
</FlatList>
) |
@DasArthur in my test cases, it looks like there is some race condition and it sometimes loads initially a large header and sometimes a small one. Can you check this solution @ArekChr @grahammendick ? |
I remember trying a variety of contentOffset and contentInset and repeatedly reloading - sometimes it worked and sometimes it didn't. My guess is that the times it worked is when the reactSetFrame was called after it was added to the window - @DasArthur Could you check if the timing of the reactSetFrame changes when it works and when it doesn't? Or does it always work for you every time you reload? |
I don't have maintainVisibleContentPosition prop, @DasArthur what version of RN are you using? |
The workaround of @alexanderstrom seems to be the only reliable one. This is where you conditionally render the ScrollView after a delay (useEffect with a timeout of 0). I think it works for the same reason that my fix works - the delayed render means that |
@grahammendick still feels so hacky :/ |
@hirbod I think another option is you could use patch-package to apply the fix to RCTScrollContentView |
@grahammendick unfortunately not an option for me, because I'm still on Expo and I don't want to eject. Since I only have 3 LargeTitle Screens, I'll try the delay render useEffect hack. |
@aryanm5 Hey, your best bet is to make your changes on the PR so Phillip can advise you of how to add the version check. But I can understand you might feel under pressure putting your code before the React Native review process. My objective-c isn't great but here's how I would've done it. - (void)didMoveToWindow
{
[super didMoveToWindow];
if (@available(iOS 14.0, *)) {
if (@available(iOS 15.0, *)) {
} else {
RCTScrollView *scrollView = (RCTScrollView *)self.superview.superview;
[scrollView updateContentOffsetIfNeeded];
}
}
}
- (void)reactSetFrame:(CGRect)frame
{
[super reactSetFrame:frame];
if (@available(iOS 14.0, *)) {
if (@available(iOS 15.0, *)) {
} else {
if (!self.window) {
return;
}
}
}
RCTScrollView *scrollView = (RCTScrollView *)self.superview.superview;
if (!scrollView) {
return;
}
RCTAssert([scrollView isKindOfClass:[RCTScrollView class]], @"Unexpected view hierarchy of RCTScrollView component.");
[scrollView updateContentOffsetIfNeeded];
} Also, make sure the formatting is consistent (use 2 spaces throughout). I wasn't bothered about this when I was patching the package, but now you're making a PR the code has to be consistent. |
iOS 15 fixed the bug I've reported initally but introduced a new one. The large header is now snapping to a small header for me as soon as I scroll, looking super weird. (Expo SDK 43, iOS 15, iPhone 13 Pro Max) |
@WoLewicki is the new introduced bug something this package can solve? Or is this still a RN Upstream issue? I really don't know how to proceed with large header if its basically broken :/ |
@hirbod do you have knowledge if such behavior happens in other libraries doing kind of the same thing, like |
@WoLewicki unfortunately not. I never tried another library. I can only tell you that it doesn’t happen in other system apps or any app I am using. |
It is probably another issue of |
@hirbod @WoLewicki The Navigation router doesn't have any problem with large headers on iOS 15. You could run my zoom sample and compare view hierarchies? |
I am having the exact same issue as @hirbod on iOS 15; glad I found this thread. I'm using |
@WoLewicki Here is a reproduction: Video
CodeRepo here: https://github.com/nandorojo/large-title-bug Click to view codeimport React from "react";
import { ScrollView, StyleSheet, Text, View } from "react-native";
import { createNativeStackNavigator } from "@react-navigation/native-stack";
import { NavigationContainer } from "@react-navigation/native";
const { Navigator, Screen } = createNativeStackNavigator();
function Home() {
return (
<ScrollView>
{new Array(40).fill(0).map((_, i) => (
<View style={[styles.item, i % 2 && styles.odd]}>{i}</View>
))}
</ScrollView>
);
}
export default function App() {
return (
<NavigationContainer>
<Navigator>
<Screen
name="Home"
component={Home}
options={{
headerLargeTitle: true,
}}
/>
</Navigator>
</NavigationContainer>
);
}
const styles = StyleSheet.create({
item: {
height: 100,
backgroundColor: "#41b87a",
},
odd: {
backgroundColor: "#333333",
},
}); |
@nandorojo thanks for providing a repro for the issue. My desk was overloaded and I haven’t managed to create one! |
@nandorojo the reproduction seems to be the same issue as the old ones with |
I don’t like the fact we need to set special props in order to make it work. It wasn’t like this till iOS 13, broken in iOS 14 and now semi working on iOS 15 (but working under special circumstances when changing header and Scrollview). Is there a way to make the DX better? I mean there is also no documentation about this. |
I am afraid these options are the defaults used by native apps on |
@WoLewicki Then I guess this issue can be closed right? Since it's not actually a bug? |
@sallar yeah, I also think there is not much to be added from the |
actually, for me, just adding |
Unfortunately, this is still an issue for me. None of the solutions in this thread have fixed this behavior. The problem seems to stem from hiding the parent navigators header and then navigating to the child screen with largeHeaderTitle set to true. |
@branaust you may have to bite the bullet a bit on this one and create your own header/navigator yourself. I had to do that on my project and spent quite a bit of time trying to get the nuances of the animation correct. However, the header behaviour is predictable after that. I am just revisiting the idea again and posted to SO https://stackoverflow.com/questions/74957469/can-a-custom-header-in-react-navigation-receive-the-scrollview-ref-in-a-createst to see if there's a saner approach than what I did. However, if you do create your own you'd have to remember the Pan Gesture to support swipe left or right on the stack or down to minimize a modal. |
@trajano I did find a bit of a hacky workaround that someone else suggested in another thread. The solution consists of rendering the scrollView after the screen has been mounted. Here's my screen:
and here is the return:
This solution does fix the largeHeaderTitle issue and now every time I navigate to the screen the large header is shown by default. In my special use case, I'm also rendering the native search bar in the header that react-navigation offers out of the box. My workaround for this was to add:
Now, when the screen is mounted, the large header works perfectly & search bar is visible (although it is always in view even when the user scrolls which is fine for now). |
@branaust what if you did
Would you eliminate the "timeout"? |
Probably not the answer you are looking for and an unpopular opinion, but if you're early in the development I can only recommend not to use React Native 😄. Use Ionic or native development instead... RN comes with tons of dependencies and people tend to use a lot of libraries. Whenever an update comes out either for the OS or React Native pretty much everything breaks. I previously tried realizing a large app project with React Native and more time was spent on maintaining, forking, manually patching libraries and investigating than the usage of RN saved in time... And nowadays in most cases web apps like Ionic feel quite native as well and save A LOT of time in development. RN is just something in between (web and native) but not doing any of both really well. |
@dioncodes I somewhat agree with you, but I also came from the hell of Ionic v2->v3->v4->aborted v5 upgrades which require me to restructure my entire code base to get things working. Working on React Native is actually less painful once you have control of the dependencies (which takes a bit of time but I think more fruitful), but Ionic upgrades require project structure changes which means you'd have to know what everyone has done and migrate over. NativeScript would've been a good option if HMR with Vue worked. Vue Native has been deprecated. Their team has focus has moved to NativeBase. The primary selling point for React Native for me is still As far as managing dependencies learning from experience, whenever we use any 3rd party dependency, we have to think of how to get rid of it asap or merge their code base into our own using yarn workspaces (haven't gotten that far myself, what I've done is clone and push to our internal npmjs with updated package.json once we verify it works correctly in the current versions of React(native). The issue also appears with Ionic though not as much because Angular tends to control everything for you much like Vue. |
Works like a charm. Thank you! |
@trajano Yeah, Expo and the previews are definitely a good point with RN. However, you also get these with Ionic or web based frameworks, because you can already preview everything in your browser including HMR... The upgrade paths for Ionic weren't that well either in history, ngl. But it's less of an issue to stick to a previous Ionic version because most is still compatible to newer OS as there are less native components. I have maintained some apps that are running in latest OS versions on a code base that is 3 years old without any migrations so far. One more reason why I stopped working with RN was that there were a few bugs with native libraries (including this react-native-screens) that no solutions existed for yet. Issues and comments were all over Github (including the official RN repo, just take a look at how many unsolved issues there are). And fixing them on your own would be a lot of work and require knowledge of the libraries or RN itself including its ancient Objective-C code. Maybe it was bad luck but during my larger RN project I stumbled in so many issues that were so time intense to solve, that it would have been much quicker to build a completely native app. Anyways I'm also not a big fan of React anymore, especially since v3 Vue became so much better and that's already another reason for me not work with RN. And Vue Native isn't a reasonable option either (as you already mentioned). I'll just stick to Vue Ionic if Android is necessary and Swift + SwiftUI otherwise. In the end everything other from completely native development is a compromise. However native Android development is such a PITA that at least for Android some web/hybrid solution is necessary (if the platform needs to be supported at all) and dedicated/separate apps for iOS and Android require a higher budget... |
Man I've figured my issue out, and it was the exact opposite of the hacky workarounds here It boiled down to 2 issues:
Making sure the FlatList is always mounted when the Screen is mounted, and making sure there are no other elements before the FlatList inside the view resolved this issue for me. I no longer have the app start with a small title, and I have no more issues with scrolling and not hiding the title |
This issue can be solved by creating your own implementation of ScrollView based on react-native APIs. https://github.com/fabOnReact/react-native-improved |
I think in my case is because the fetch data was too fast from local server I tested with a setTimeOut in the await function from the refetching to refresh and the auto-release won't happened |
Hi,
I just upgraded to Expo SDK 39 and before of that, I had never an issue navigating to a child screen (from LargeHeader to small header). When I go back now, it stays smalls instead of growing back to large.
What has changed and is this something known?
The text was updated successfully, but these errors were encountered: