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

Large title: The navbar gets stuck when using the refresh control. #1034

Closed
maxencehenneron opened this issue Aug 3, 2021 · 9 comments
Closed

Comments

@maxencehenneron
Copy link

maxencehenneron commented Aug 3, 2021

Description

When using a refreshcontrol, large titles and a scrollview, the navbar gets stuck when refreshing.

This seems to be related to this: https://stackoverflow.com/questions/50708081/prefer-large-titles-and-refreshcontrol-not-working-well.

It seems like the scrollview needs to be at the top of the main view for this to work correctly, which is currently not possible with this library

Other bugs I found:

Screenshots

Simulator.Screen.Recording.-.iPhone.11.-.2021-08-03.at.15.11.36.mp4

Steps To Reproduce

  1. Add a native stack container
  2. Set "headerLargeTitle" to true in the screenOptions
  3. Add a refresh control
  4. Run the app, refresh.

Expected behavior

It should snap back to its initial position

Actual behavior

It gets stuck when refreshing stops.

Snack or minimal code example

https://github.com/maxencehenneron/RefreshControlBug/blob/main/App.js
(You can pull the repository to test)

Package versions

  • React: 17.0.1
  • React Native: 0.64.2
  • React Native Screens: 3.4.0
@maxencehenneron
Copy link
Author

maxencehenneron commented Aug 3, 2021

I found a fix for this but it's dirty:

In RNScreen.m, add the following in the viewDidLayoutSubviews function:

UIViewController *parentVC = [self parentViewController];
if ([parentVC isKindOfClass:[UINavigationController class]]) {
  [((UINavigationController *)parentVC).navigationBar sizeToFit];
}

This ensures the navigation bar re-renders when the screen updates... Any thoughts?
Final result:

Simulator.Screen.Recording.-.iPhone.11.-.2021-08-03.at.16.09.28.mp4

@WoLewicki
Copy link
Member

Is it a duplicate of #395 ?

@maxencehenneron
Copy link
Author

maxencehenneron commented Aug 4, 2021

Not really. This bug was related to React Native 0.63. facebook/react-native#28236 was supposed to resolve it, but with react-native-screens, it doesn't seem to be the case. It actually made it worse.

The refresh control is not hidden under the navbar anymore, but the navbar gets stuck when setting the RefreshControl "refreshing" prop from true to false. The scroll view seems to snap back to its correct position but the navbar inside of the UINavigationController does not.

By forcing the navbar to update everytime there's a layout change, it does resolve the issue, but it's a dirty hack. I'm not sure why this is happening at this time.

I made a repo to reproduce this bug: https://github.com/maxencehenneron/RefreshControlBug

@WoLewicki
Copy link
Member

In your repro you did not provide contentInsetAdjustmentBehavior=“automatic” on the ScrollView, but on the RefreshControl and there is no headerTranslucent: true option on the stack. These settings are necessary for this to work with the ScrollView as mentioned in #395 (comment). After applying them, the repro seems to work correctly. Can you confirm that?

@maxencehenneron
Copy link
Author

maxencehenneron commented Aug 17, 2021

Hi @WoLewicki, sorry I was on vacation last week. Thank you for your help! It does seem to work when setting the header to translucent. However it makes the header, well, translucent. I can set the header background color to white to fix it, but maybe it's worth investigating why this is happening?

Edit: I found the code that is likely causing this issue:

if (!shouldHide && !config.translucent) {

I wonder if there's a better way to avoid system laying out the screen underneath navigation controllers without setting edgesForExtendedLayout = UIRectEdgeNone.

Or we could specify it in the native stack documentation somewhere

@WoLewicki
Copy link
Member

I think since the translucent header is the default option in native apps, maybe we should make the header translucent by default on iOS. I am also not sure what you would want to achieve. If you want the refreshControl to appear in the area of the header, it seems reasonable that this area is available for laying anything underneath it. Can you elaborate more on it?

@maxencehenneron
Copy link
Author

Yes, I agree, translucent should be set to TRUE by default, especially when using large titles.
I'm able to achieve what I want to do by setting translucent to true and the header background color to white.

My comment was about these lines in the library source code:

if (!shouldHide && !config.translucent) {
// when nav bar is not translucent we chage edgesForExtendedLayout to avoid system laying out
// the screen underneath navigation controllers
vc.edgesForExtendedLayout = UIRectEdgeNone;
} else {
// system default is UIRectEdgeAll
vc.edgesForExtendedLayout = UIRectEdgeAll;
}

I was wondering if we could make it work without needing to set translucent to true

@WoLewicki
Copy link
Member

I think it might be good to change it to be set vc.edgesForExtendedLayout = UIRectEdgeAll; by default when you have large title, and change it to UIRectEdgeNone when headerTranslucent is explicitly set to false. We are always open to PRs if you would like to do it.

@WoLewicki
Copy link
Member

WoLewicki commented Sep 10, 2021

I will close this issue since I think there is nothing to be added here. Feel free to comment if I am wrong and we can reopen it then.

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

No branches or pull requests

2 participants