-
-
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
fix: large header sizeToFit #670
Conversation
@nickyleach can you try and use the solution proposed here: #645 (comment)? |
@WoLewicki the proposed solution is working for me. |
@WoLewicki how can we help to merge it? We have to patch it in out code... |
@savelichalex I am afraid the change proposed here was rather a quick workaround, not a solution to the problem, and so it might not be ever merged in order not to introduce any other bugs. I think it is better to apply #645 (comment) as it seems to consistently solve the problem. |
@WoLewicki the thing is we apply both that workarounds to get proper behaviour. If you look at gif above, you may notice that when it go back to Should I open a new issue for that? Or there are other ways how we could solve it? We are ready to contribute. |
I still don't get why you need this PR. #645 (comment) answers the issue of a folded header and this PR tries to do the same thing exactly. |
@WoLewicki but what about Expo users? We need a native fix shipped with react-native-screens (if possible) to be included in SDK 41. Patching isn't an option for folks like me, because I love the managed experience. (Even though downsides are that I am sometimes handcuffed) |
This PR is not a solution to the problem, rather a workaround, which can result in other bugs, so we most probably don't want to merge it unfortunately. I believe the real solution is the change of |
None of them worked for me @WoLewicki |
I have no easy solution for this then unfortunately. Probably when EAS comes live for everyone, it will be simpler to patch native code for managed workflow. |
I am afraid not, only the one from this PR and the ones from the issues. |
@WoLewicki I think this problem is already solved in screens (I've also double-checked Test1072 and I see that large header is expanded. This is possibly working because of this line - react-native-screens/ios/RNSScreenStack.mm Line 160 in 9fbab8d
|
Applied https://developer.apple.com/forums/thread/660745 solution suggested by @julian-baumann to make the large header appear on the Screen load. Should resolve #649 and #645.