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

Cherry-pick: Fix contentInsetAdjustmentBehavior set to automatic on ScrollView in the new architecture (#34217) #1484

Merged
merged 1 commit into from
Nov 11, 2022

Conversation

christophpurrer
Copy link

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

Cherry-pick a react-native@0.71 commit into the current main branch:
facebook@27fe6f10796

Fixes facebook#34165 and Large title fails on the new React Native architecture.

There are problems with setting contentInsetAdjustmentBehavior to automatic on the ScrollView component in the new React Native architecture. The automatic setting matters to navigation libraries (like my Navigation router) because it stops the ScrollView from overlapping the UINavigationBar. The setting also powers important native features like large titles and search bars on iOS.

The automatic setting works fine on the old architecture. It doesn’t work on the new architecture because React Native is recycling views. In facebook#34165 and Large title fails there are videos comparing the setting in the old and the new architecture.

Changelog

[iOS] [Fixed] - Fix contentInsetAdjustmentBehavior set to automatic on ScrollView in the new architecture

Test Plan

I checked the fix in both the repros in facebook#34165 and Large title fails. Here is a video of the fix running with large titles in the new architecture.

Screen.Recording.2022-07-18.at.21.31.11.mov

@christophpurrer christophpurrer requested a review from a team as a code owner November 11, 2022 12:19
…the new architecture (facebook#34217)

Summary:
Fixes facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43) on the new React Native architecture.

There are problems with setting `contentInsetAdjustmentBehavior` to `automatic` on the `ScrollView` component in the new React Native architecture. The `automatic` setting matters to navigation libraries (like [my Navigation router](https://github.com/grahammendick/navigation)) because it stops the `ScrollView` from overlapping the `UINavigationBar`. The setting also powers important native features like large titles and search bars on iOS.

The `automatic` setting works fine on the old architecture. It doesn’t work on the new architecture because React Native is recycling views. In facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43) there are videos comparing the setting in the old and the new architecture.

## Changelog

[iOS] [Fixed] - Fix `contentInsetAdjustmentBehavior` set to `automatic` on `ScrollView` in the new architecture

Pull Request resolved: facebook#34217

Test Plan:
I checked the fix in both the repros in facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43). Here is a video of the fix running with large titles in the new architecture.

https://user-images.githubusercontent.com/1761227/179612188-162b896b-82c5-45de-bb5a-ba80f452fbee.mov

Reviewed By: sammy-SC

Differential Revision: D37952506

Pulled By: cipolleschi

fbshipit-source-id: 6cff6c85aa33b579405fe34a9e36c8630f4c24bd
@Saadnajmi Saadnajmi merged commit 7579db9 into microsoft:main Nov 11, 2022
@christophpurrer christophpurrer deleted the fixContentInsent branch November 11, 2022 22:33
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Feb 13, 2023
…the new architecture (facebook#34217) (microsoft#1484)

Summary:
Fixes facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43) on the new React Native architecture.

There are problems with setting `contentInsetAdjustmentBehavior` to `automatic` on the `ScrollView` component in the new React Native architecture. The `automatic` setting matters to navigation libraries (like [my Navigation router](https://github.com/grahammendick/navigation)) because it stops the `ScrollView` from overlapping the `UINavigationBar`. The setting also powers important native features like large titles and search bars on iOS.

The `automatic` setting works fine on the old architecture. It doesn’t work on the new architecture because React Native is recycling views. In facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43) there are videos comparing the setting in the old and the new architecture.

## Changelog

[iOS] [Fixed] - Fix `contentInsetAdjustmentBehavior` set to `automatic` on `ScrollView` in the new architecture

Pull Request resolved: facebook#34217

Test Plan:
I checked the fix in both the repros in facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43). Here is a video of the fix running with large titles in the new architecture.

https://user-images.githubusercontent.com/1761227/179612188-162b896b-82c5-45de-bb5a-ba80f452fbee.mov

Reviewed By: sammy-SC

Differential Revision: D37952506

Pulled By: cipolleschi

fbshipit-source-id: 6cff6c85aa33b579405fe34a9e36c8630f4c24bd

Co-authored-by: Graham Mendick <graham.mendick@gmail.com>
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Mar 10, 2023
…the new architecture (facebook#34217) (microsoft#1484)

Summary:
Fixes facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43) on the new React Native architecture.

There are problems with setting `contentInsetAdjustmentBehavior` to `automatic` on the `ScrollView` component in the new React Native architecture. The `automatic` setting matters to navigation libraries (like [my Navigation router](https://github.com/grahammendick/navigation)) because it stops the `ScrollView` from overlapping the `UINavigationBar`. The setting also powers important native features like large titles and search bars on iOS.

The `automatic` setting works fine on the old architecture. It doesn’t work on the new architecture because React Native is recycling views. In facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43) there are videos comparing the setting in the old and the new architecture.

## Changelog

[iOS] [Fixed] - Fix `contentInsetAdjustmentBehavior` set to `automatic` on `ScrollView` in the new architecture

Pull Request resolved: facebook#34217

Test Plan:
I checked the fix in both the repros in facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43). Here is a video of the fix running with large titles in the new architecture.

https://user-images.githubusercontent.com/1761227/179612188-162b896b-82c5-45de-bb5a-ba80f452fbee.mov

Reviewed By: sammy-SC

Differential Revision: D37952506

Pulled By: cipolleschi

fbshipit-source-id: 6cff6c85aa33b579405fe34a9e36c8630f4c24bd

Co-authored-by: Graham Mendick <graham.mendick@gmail.com>
shwanton pushed a commit to shwanton/react-native-macos that referenced this pull request Mar 10, 2023
…the new architecture (facebook#34217) (microsoft#1484)

Summary:
Fixes facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43) on the new React Native architecture.

There are problems with setting `contentInsetAdjustmentBehavior` to `automatic` on the `ScrollView` component in the new React Native architecture. The `automatic` setting matters to navigation libraries (like [my Navigation router](https://github.com/grahammendick/navigation)) because it stops the `ScrollView` from overlapping the `UINavigationBar`. The setting also powers important native features like large titles and search bars on iOS.

The `automatic` setting works fine on the old architecture. It doesn’t work on the new architecture because React Native is recycling views. In facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43) there are videos comparing the setting in the old and the new architecture.

## Changelog

[iOS] [Fixed] - Fix `contentInsetAdjustmentBehavior` set to `automatic` on `ScrollView` in the new architecture

Pull Request resolved: facebook#34217

Test Plan:
I checked the fix in both the repros in facebook#34165 and [Large title fails](reactwg/react-native-new-architecture#43). Here is a video of the fix running with large titles in the new architecture.

https://user-images.githubusercontent.com/1761227/179612188-162b896b-82c5-45de-bb5a-ba80f452fbee.mov

Reviewed By: sammy-SC

Differential Revision: D37952506

Pulled By: cipolleschi

fbshipit-source-id: 6cff6c85aa33b579405fe34a9e36c8630f4c24bd

Co-authored-by: Graham Mendick <graham.mendick@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

ScrollView doesn't reset inset when contentInsetAdjustmentBehavior is automatic
3 participants