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.

Sign up for GitHub

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

Jump to bottom

contentInsets always overwrites scrollIndicatorInsets of ScrollView/ListView #13025

Closed
allenhsu opened this issue Mar 20, 2017 · 5 comments
Closed

contentInsets always overwrites scrollIndicatorInsets of ScrollView/ListView #13025

allenhsu opened this issue Mar 20, 2017 · 5 comments
Assignees
@shergin
Labels
Bug Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Platform: iOS iOS applications. Resolution: Fixed A PR that fixes this issue has been merged. Resolution: Locked This issue was locked by the bot.

Comments

@allenhsu
Copy link

Description

I set contentInsets and scrollIndicatorInsets of a ScrollView, actually ListView, to different values. But seems the value of contentInsets always overwrites the scrollIndicatorInsets one.

Reproduction

https://sketch.expo.io/Byi2M7psg

In this demo, scrollIndicatorInsets.top is 20, it's expected to be 100. After deleting contentInset={{top: 20, left: 0, right: 0, bottom: 0}}, the scrollIndicatorInsets is correct.

Solution

On line 262 of https:/facebook/react-native/blob/master/React/Views/RCTView.m#L262 scrollIndicatorInsets is set to baseInset which is contentInset most of the time. This method is called when setContentInset for RCTScrollView.

On line 372 of https:/facebook/react-native/blob/v0.42.3/React/Views/RCTComponentData.m#L372 when enumerate props, the order should not be predictable, but actually contentInset always comes after scrollIndicatorInsets during my debugging. So contentInset always overwrites scrollIndicatorInsets unless it's Zero (returned on if (UIEdgeInsetsEqualToEdgeInsets(contentInset, _contentInset)) return).

Additional Information

  • React Native version: 0.42.3
  • Platform: iOS
  • Operating System: MacOS
  • Dev tools: iOS 10.2.1
@janicduplessis janicduplessis added Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Platform: iOS iOS applications. labels Mar 20, 2017
@shergin shergin self-assigned this Mar 20, 2017
@hramos
Copy link
Contributor

hramos commented May 31, 2017

Thanks for reporting this. Would you consider sending a PR? It seems like you've already done some of the work here.

@hramos hramos added the Good first issue Interested in collaborating? Take a stab at fixing one of these issues. label Aug 4, 2017
timotew added a commit to timotew/react-native that referenced this issue Dec 31, 2017
@timotew
Fix: Avoid contentInsets overwrites scrollIndicatorInsets on iOS face…
d1b9d32 
…book#13025

Fix to avoid contentInsets always overwrites scrollIndicatorInsets of ScrollView/ListView.
According to issue facebook#13025 https:/facebook/react-native/blob/a8391bde7d757d01521a6d12170fb9090c17a6a0/React/Views/RCTView.m#L299
always overide `scrollIndicatorInsets` when called and only works when `contentInsets` is not set.
We only need to worry about when `contentInsets` is set, We only have to check and reset it after `autoAdjustInsetsForView` is called.
@timotew timotew mentioned this issue Dec 31, 2017
Closed
@gastonmorixe
Copy link

Any update?

@ghost
Copy link

ghost commented Oct 10, 2018 •
edited by ghost
Loading

The solution given in the description have been implemented in a previous commit. Does this issue still persist ?

@hramos hramos removed the Bug Report label Feb 6, 2019
@cpojer cpojer mentioned this issue Mar 19, 2019
Closed
@Salakar
Copy link
Contributor

Salakar commented Apr 10, 2019

👋 hey all, thanks for reporting this issue.

There was a commit some time ago (timotew@d1b9d32) that was meant to have fixed this issue, because of this I'm going to go ahead and close this issue for now.

Should someone be able to provide a reproduction on the latest version of React Native I'd be happy to re-open this, please let me know.

Thank you

@Salakar Salakar closed this as completed Apr 10, 2019
@Salakar Salakar added Resolution: Fixed A PR that fixes this issue has been merged. Resolution: Needs Repro and removed Good first issue Interested in collaborating? Take a stab at fixing one of these issues. labels Apr 10, 2019
@gabrielbull
Copy link
Contributor

gabrielbull commented Jul 12, 2019 •
edited
Loading

@Salakar This bug is back, the code from the PR seems to have disappeared. Can you open the issues again?

@timotew You provided the PR to fix this issue. Maybe you can help us understand why it's back?

@facebook facebook locked as resolved and limited conversation to collaborators Apr 10, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Apr 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@shergin shergin

Labels
Bug Issue: Author Provided Repro This issue can be reproduced in Snack or an attached project. Platform: iOS iOS applications. Resolution: Fixed A PR that fixes this issue has been merged. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
8 participants
@shergin @hramos @allenhsu @gastonmorixe @gabrielbull @janicduplessis @Salakar @react-native-bot