Summary:
Content offset was broken because on initial render contentSize is {0,0} so any positive offset is lost. Also inset top/bottom and left/right were inversed �, this led to bad initial scrolling offset when using contentInset. This fixes it by making sure contentSize is actually measured (not {0,0}. I guess it's possible that the content is ACTUALLY {0,0} but in that case I don't think it really matters).
**Test plan**
Tested that a scrollview has proper scroll position when specifying contentOffset. Also tested that it works well with contentInset.
```js
<ScrollView contentOffset={{y: 100}}>
<View style={{height: 1000}} />
</ScrollView>
```
Closes https://github.com/facebook/react-native/pull/15670
Differential Revision: D5771221
Pulled By: shergin
fbshipit-source-id: 455ed8fd5a4ad1ec61780b573d1a8ef1d77dd124
Summary:
> The property contentInset can change the maximum and minimum values of the content offset to allow scrolling outside of the scrollable area. Its type is UIEdgeInsets, which consists of 4 numbers: {top, left, bottom, right}. When you introduce an inset, you change the range of the content offset. For example, setting the content inset to have a value of 10 for its top value allows the content offset’s y value to reach -10. This introduces padding around the scrollable area.
( https://www.objc.io/issues/3-views/scroll-view/ )
See also: https://github.com/facebook/react-native/pull/15395
Reviewed By: mmmulani
Differential Revision: D5607192
fbshipit-source-id: 1acd6a84e2bcfefc6e82861cfbdfe6247d0e4264
Summary:
When false, ScrollView disables use of pinch gestures to zoom in and out. This allows ScrollView's pinch gesture responder to be disabled to only allow zooming programmatically. The default value is ~false~ true.
**Test Plan**
Tested that pinch gesture responder is disabled when pinchEnabled=false.
/cc nicklockwood sahrens
🍺
Closes https://github.com/facebook/react-native/pull/10037
Differential Revision: D5491953
Pulled By: shergin
fbshipit-source-id: eae16f92ec616e415b4ddacfccb84c697582daf9
Summary:
In iOS11, Apple added a new layout feature called "Safe Areas" (this blog post talks a bit about it: https://www.bignerdranch.com/blog/wwdc-2017-large-titles-and-safe-area-layout-guides/).
UIScrollView is one component that is affected by this change in Apple's API. When the `contentInsetAdjustmentBehavior` is set to `automatic`, for example, it will adjust the insets (and override any manually set insets) automatically based on whether or not there's a UINavigationBar, a UITabBar, a visible status bar, etc on the screen. Frustratingly, Apple decided to default to `Automatic` for this behavior, which will cause any apps that set contentInset/contentContainerStyle padding to have their values offset by, at the very least, the size of the status bar, when they compile their app for iOS 11. Here's more information about this behavior: https://developer.apple.com/documentation/uikit/uiscrollview/2902261-contentinsetadjustmentbehavior?language=objc
Mostly, this is a really straightforward change -- it simply adds a new iOS-only prop to ScrollView that allows setting `contentInsetAdjustmentBehavior`. But I did decide to default the behavior to `never`, so that it mimics the behavior we've seen in iOS < 11. I think it's good to keep something as crucial as scrollview content insets non-magical, and also keep it behaving similarly between platforms.
Closes https://github.com/facebook/react-native/pull/15023
Reviewed By: javache
Differential Revision: D5517552
Pulled By: hramos
fbshipit-source-id: c9ce4bf331b3d243228268d826fdd4dcee99981d
Summary:
In iOS11, Apple added a new layout feature called "Safe Areas" (this blog post talks a bit about it: https://www.bignerdranch.com/blog/wwdc-2017-large-titles-and-safe-area-layout-guides/).
UIScrollView is one component that is affected by this change in Apple's API. When the `contentInsetAdjustmentBehavior` is set to `automatic`, for example, it will adjust the insets (and override any manually set insets) automatically based on whether or not there's a UINavigationBar, a UITabBar, a visible status bar, etc on the screen. Frustratingly, Apple decided to default to `Automatic` for this behavior, which will cause any apps that set contentInset/contentContainerStyle padding to have their values offset by, at the very least, the size of the status bar, when they compile their app for iOS 11. Here's more information about this behavior: https://developer.apple.com/documentation/uikit/uiscrollview/2902261-contentinsetadjustmentbehavior?language=objc
Mostly, this is a really straightforward change -- it simply adds a new iOS-only prop to ScrollView that allows setting `contentInsetAdjustmentBehavior`. But I did decide to default the behavior to `never`, so that it mimics the behavior we've seen in iOS < 11. I think it's good to keep something as crucial as scrollview content insets non-magical, and also keep it behaving similarly between platforms.
Closes https://github.com/facebook/react-native/pull/15023
Differential Revision: D5441491
Pulled By: shergin
fbshipit-source-id: 7b56ea290f7f6eca5f1d996ff8488f40b866c2e6
Summary:
<!--
Thank you for sending the PR!
If you changed any code, please provide us with clear instructions on how you verified your changes work. In other words, a test plan is *required*. Bonus points for screenshots and videos!
Please read the Contribution Guidelines at https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md to learn more about contributing to React Native.
Happy contributing!
-->
Closes https://github.com/facebook/react-native/pull/15156
Differential Revision: D5479265
Pulled By: shergin
fbshipit-source-id: a2dfa3a4357e126838a17dac4797d1d845cd56ae
Summary:
* Now `setFrame:` is called by autoresizing masks, so it is safe.
* Nobody calls `setBounds:`, so it is also safe.
Reviewed By: javache
Differential Revision: D5414441
fbshipit-source-id: 6fc51c7598c4817301db51f627aa1e9840642fec
Summary:
Previous `contentOffset` can be invalid for a new layout and overscroll the ScrollView, so the diff fixes that.
Also documented here: https://github.com/facebook/react-native/issues/13566
Reviewed By: mmmulani
Differential Revision: D5414442
fbshipit-source-id: 7de1b4a4571108a37d1795e80f165bca5aba5fef
Summary:
Surprisingly enough, even if semantically the code remains identical, layouting via autoresizing masks applies changes to subviews a bit earlier than iOS calls `layoutSubviews`.
This allows us to avoid situations where we already explicitly set calculated by Yoga frames and want to scroll to some subview, but actual layout have not done yet and internal views has wrong frames.
Reviewed By: javache
Differential Revision: D5414440
fbshipit-source-id: d4152c9c68dc17f6827832dcb45e5ba86fb82831
Summary:
This PR fixes#15006 by removing all UI API calls from RCTScrollEvent.
`-[RCTScrollEvent arguments]` can now be called from a background thread.
The Main Thread Checker of Xcode 9 will not any longer produce runtime issues when calling this method.
1. create a React Native (version: this PR) project with a scroll view
2. open it in Xcode 9
3. launch it
4. scroll the scroll view
5. observe the runtime issues in Xcode. There should not contain "UI API called from background thread"-issues.
I verified my changes on this branch: https://github.com/HeEAaD/Demo-ReactNative-UI-not-on-main-thread/tree/fix
<!--
Thank you for sending the PR!
If you changed any code, please provide us with clear instructions on how you verified your changes work. In other words, a test plan is *required*. Bonus points for screenshots and videos!
Please read the Contribution Guidelines at https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md to learn more about contributing to React Native.
Happy contributing!
-->
Closes https://github.com/facebook/react-native/pull/15008
Differential Revision: D5424734
Pulled By: shergin
fbshipit-source-id: 56beec2d7603ea6782d55622567509f3758a4517
Summary:
Thanks for submitting a pull request! Please provide enough information so that others can review your pull request:
> **Unless you are a React Native release maintainer and cherry-picking an *existing* commit into a current release, ensure your pull request is targeting the `master` React Native branch.**
Explain the **motivation** for making this change. What existing problem does the pull request solve?
The problem occurs when a ScrollView's content height is smaller than the ScrollView height. If the method `scrollToEnd` is called on the ScrollView, it will pull the content down until the bottom of the content is aligned with the bottom of the Scrollview container.
This fix will ensure the proper functionality: That the furthest the ScrollView can scroll down is to where the top of the content container is at the origin (i.e., the ScrollView scroll number cannot be less than 0).
Prefer **small pull requests**. These are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.
**Test plan (required)**
Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI.
Make sure tests pass on both Travis and Circle CI.
I tested on a scenario where the ScrollView is almost the full size of the screen, and the content of the ScrollView has a height of much less. In this situation, the `scrollToEnd` method was executed and the content stayed in the same position. This is the intended behavior. If the content of the ScrollView is smaller than the height of the ScrollView, then the `scrollToEnd` method should not scroll anywhere.
**Code formatting**
Look around. Match the style of the rest of the codebase. See also the simple [style guide](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#style-guide).
For more info, see the ["Pull Requests" section of our "Contributing" guidelines](https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#pull-requests).
Closes https://github.com/facebook/react-native/pull/12889
Reviewed By: javache
Differential Revision: D5289894
Pulled By: sahrens
fbshipit-source-id: df2e779ee855c1dea85d33649d754371ad244bca
Summary: Optimize ScrollView by adding flag "DEPRECATED_sendUpdatedChildFrames" to gate whether updatedChildFrames data is computed and propagated on scroll events. The frame data is used in ListView by the onChangeVisibleRows prop. When this prop is not defined, unnecessary computation in ScrollView should not be performed.
Reviewed By: sahrens
Differential Revision: D5174898
fbshipit-source-id: e3eaed8760b76becf14dfeb00122bdebdaeae4ef
Summary:
fix problem of function scrollToEnd: There are some strange thing happened when contentSize.height(width) is smaller than bounds.size.height(width). In fact, there is no need to scroll in this case.
Thanks for submitting a PR! Please read these instructions carefully:
- [x] Explain the **motivation** for making this change.
- [x] Provide a **test plan** demonstrating that the code is solid.
- [x] Match the **code formatting** of the rest of the codebase.
- [x] Target the `master` branch, NOT a "stable" branch.
What existing problem does the pull request solve?
A good test plan has the exact commands you ran and their output, provides screenshots or videos if the pull request changes UI or updates the website. See [What is a Test Plan?][1] to learn more.
If you have added code that should be tested, add tests.
Sign the [CLA][2], if you haven't already.
Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.
Make sure all **tests pass** on both [Travis][3] and [Circle CI][4]. PRs that break tests are unlikely to be merged.
For more info, see the ["Pull Requests"][5] section of our "Contributing" guidelines.
[1]: https://medium.com/martinkonicek/what-is-a-test-plan-8bfc840ec171#.y9lcuqqi9
[2]: https://code.facebook.com/cla
[3]: https://travis-ci.org/facebook/react-native
[4]: http://circleci.com/gh/facebook/react-native
[5]: https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#pull-requests
Closes https://github.com/facebook/react-native/pull/13180
Differential Revision: D4928778
Pulled By: javache
fbshipit-source-id: 8b74833593ee317df726a4035ec71bbc77d13afe
Summary:
Remove the native iOS sticky headers implementation that has been replaced by the js Animated one. Also remove a line in JS that made sure we passed null to native so it did not use the native implementation.
**Test plan**
Made sure there were no more mentions of sticky / header in native ScrollView related code.
Tested that sticky headers still work :o
Closes https://github.com/facebook/react-native/pull/12696
Differential Revision: D4657391
Pulled By: ericvicenti
fbshipit-source-id: 16324a45ca4ce5cd143293c61394a0fa7ad0c4a1
Summary:
To make React Native play nicely with our internal build infrastructure we need to properly namespace all of our header includes.
Where previously you could do `#import "RCTBridge.h"`, you must now write this as `#import <React/RCTBridge.h>`. If your xcode project still has a custom header include path, both variants will likely continue to work, but for new projects, we're defaulting the header include path to `$(BUILT_PRODUCTS_DIR)/usr/local/include`, where the React and CSSLayout targets will copy a subset of headers too. To make Xcode copy headers phase work properly, you may need to add React as an explicit dependency to your app's scheme and disable "parallelize build".
Reviewed By: mmmulani
Differential Revision: D4213120
fbshipit-source-id: 84a32a4b250c27699e6795f43584f13d594a9a82
Summary: Reveting the recent view clipping changes, since it doesn't work well with modals and the fix is not super simple.
Reviewed By: mmmulani
Differential Revision: D4204490
fbshipit-source-id: 510f2b04c604b3f3a223dc4accb424b030876fbe
Summary:
UIScrollView's internal logic with scroll indicator dies when bad values (e.g. NaN/Infinity) are set on the position/size.
We already guard UIManager with these checks but the RCTScrollView's underlying scrollview (RCTCustomScrollView) can get these set from other places, and we're seeing crashes in this area.
Reviewed By: javache
Differential Revision: D4088601
fbshipit-source-id: b1185cc7c65ba0266787441169264c94338fc55c
Summary:
First commit for Apple TV support: changes to existing Objective-C code so that it will compile correctly for tvOS.
Closes https://github.com/facebook/react-native/pull/9649
Differential Revision: D3916021
Pulled By: javache
fbshipit-source-id: 34acc9daf3efff835ffe38c43ba5d4098a02c830
Summary: We deprecated it a while back and nobody is using it internally.
Reviewed By: majak
Differential Revision: D3542602
fbshipit-source-id: dfe11a47b21d2f8a7c946c902f0ea427615ffc31
Summary:
Issue we were seeing: scrollview would clip its cells when it was resized to 0 height and moved offscreen, but it wouldn't add it back when it was resized and moved back
Why this was happening: scrollview wouldn't rerun its un/clipping logic after the first run unless 1/it has 0x0 frame or 2/it has been scrolled. Neither was happening here.
Fix: run the un/clipping logic when scrollview's frame has been changed since the last clipping.
Reviewed By: javache
Differential Revision: D3436996
fbshipit-source-id: 1a8cfeb72b425fcc80815d30743fa308b9c75ab6
Summary:
This diff refactors the view update process into two stages:
1. The `reactSubviews` array is set, whose order matches the order of the JS components and shadowView components, as specified by the UIManager.
2. The `didUpdateReactSubviews` method is called, which actually inserts the reactSubviews into the view hierarchy.
This simplifies a lot of the hacks we had for special-case treatment of subviews: In many cases we don't want to actually insert `reactSubviews` into the parentView, and we had a bunch of component-specific solutions for that (typically overriding all of the reactSubviews methods to store views in an array). Now, we can simply override the `didUpdateReactSubviews` method for those views to do nothing, or do something different.
Reviewed By: wwjholmes
Differential Revision: D3396594
fbshipit-source-id: 92fc56fd31db0cfc66aac3d1634a4d4ae3903085
Summary:
RCTScrollView was calling `dockClosestSectionHeader` in `reactBridgeDidFinishTransaction`, which triggers a layout update. The reason for this was in case the `stickyHeaderIndexes` property was updated, which would require the headers to be adjusted.
However, doing this in `reactBridgeDidFinishTransaction` had the affect of causing `layoutSubviews` to be called repeatedly every frame even if nothing had changed and the scrollview wasn't moving, which was especially expensive when combined with the `removeClippedSubviews` logic, that loops through every view to calculate if it needs to be clipped.
This fix moves the `dockClosestSectionHeader` call into `didUpdateProps`, and checks that the stickyHeaderIndexes have actually changed before calling it.
Reviewed By: javache
Differential Revision: D3387607
fbshipit-source-id: c71e00c6fac48337a63d7fee7c7c23e016acf24e
Summary:
Removes the deprecated APIs that were replaced by `RefreshControl`. Those API have been deprecated for a while already so I think it's fine to remove them at this point. Also ported the `SwipeRefreshLayoutTestModule` test to use `RefreshControl` instead of `PullToRefreshViewAndroid`.
**Test plan (required)**
Made sure no references are left in the codebase to `PullToRefreshViewAndroid`, `onRefreshStart` and `endRefreshing`.
Tested that `ScrollView` examples in UIExplorer still work properly.
Check that the `SwipeRefreshLayoutTestModule` passes on CI.
Closes https://github.com/facebook/react-native/pull/7447
Reviewed By: mkonicek
Differential Revision: D3292391
Pulled By: bestander
fbshipit-source-id: 27eb2443861e04a9f7319586ce2ada381b714d47
Summary: We are deprecating nativeScrollDelegate property in RCTScrollableProtocol in favor of a scrollListener api. To register/unregister from scroll events use the addScrollListener/removeScrollListener methods
Reviewed By: javache
Differential Revision: D3278218
fbshipit-source-id: 54373cae8e9f8efa7cdbd40c51bcf21d368acf75
Summary:
When throttling scroll events with `scrollEventThrottle`, `onScroll`
is not guaranteed to be fired for the final scroll position
of the `ScrollView`. This can cause a component to render UI that
is consistent with the resting scroll position of the `ScrollView`.
This commit guarantees that an `onScroll` event will be fired for
the resting scroll position of the `ScrollView`.
**Test plan (required)**
Verified commit fixes a reduced repro. Also tested fix in a larger app.
Adam Comella
Microsoft Corp.
Closes https://github.com/facebook/react-native/pull/7366
Differential Revision: D3269303
Pulled By: javache
fb-gh-sync-id: f68ecb7e9c18d1ac255c6f872fb7eb4aadd07799
fbshipit-source-id: f68ecb7e9c18d1ac255c6f872fb7eb4aadd07799
Summary: Using customDirectEventTypes or customBubblingEventTypes causes a viewmanager to be initialized at app start. This diff deprecates those methods and removes their usage from RCTScrollViewManager.
Reviewed By: majak
Differential Revision: D3218973
fb-gh-sync-id: 295bef3be9623b49b0cdcbf8a56e10d9b28126d9
fbshipit-source-id: 295bef3be9623b49b0cdcbf8a56e10d9b28126d9
Summary:A need for sending a scroll events outside of scrollview made D3092854 a bit clunky. This diff kinda fixes it by tightening up emitting of fake scroll events just to the only usecase we have right now.
Why not just simply construct the event in `RCTNavigator`, so we can drop the code from `RCTScrollView` altogether?
`RCTScrollEvent` is private to `RCTScrollView`, and that's good. We don't want anyone have an ability to make up scroll events. Even this existing functionality should be sunset one day when we better integrate with native gesture recognizers.
Depends on D3092867.
Reviewed By: javache
Differential Revision: D3120751
fb-gh-sync-id: 6519c055b983cfd48c4b4a9d619c4452e12efda1
fbshipit-source-id: 6519c055b983cfd48c4b4a9d619c4452e12efda1
Summary: This was previously removed in D2884587, but we will need it going forward. See D3092867 for reasons why it's necessary again.
Reviewed By: javache
Differential Revision: D3092848
fb-gh-sync-id: 0d10dbac4148fcc8e035d32d8eab50f876d99e88
fbshipit-source-id: 0d10dbac4148fcc8e035d32d8eab50f876d99e88
Summary:In order to ensure that the docked sticky header in a ListView receives touches correctly, RCTScrollView has a custom hitTest implementation that checks the sticky headers for touches prior to checking any other views.
There was a bug in this implementation that meant that sticky views would get touch priority even if the touch was outside the bounds of the scrollView. This meant that sticky headers that scrolled off the top of the list would intercept touches intended for views placed above the scrollView.
This diff fixes that bug by checking that the touch is inside the scrollview before checking for sticky header touches. I've also limited the custom hit test logic to just the currently docked header, as the other sticky header views do not require special treatment.
Reviewed By: javache
Differential Revision: D3041236
fb-gh-sync-id: a2a3474dda03d5b51688bd575195a67956184bbe
shipit-source-id: a2a3474dda03d5b51688bd575195a67956184bbe
Summary:Currently, an empty `<ScrollView />` on iOS always throws the warning "Sticky header index 0 was outside the range {0, 0}".
This is because the error-reporting code relies on the assumption that `stickyHeaderIndices` exists, and when it doesn't the error check thinks there's an index when there really isn't.
Note that this only changes error reporting and won't affect apps out of debug mode.
**Test plan**
I created a sample app and included an empty `<ScrollView />`. Without this change the "Sticky header..." warning was displayed on every run through. With this change implemented, the warning went away.
Closes https://github.com/facebook/react-native/pull/6417
Differential Revision: D3042178
Pulled By: nicklockwood
fb-gh-sync-id: 7afefd428a5fcb03867bcb69e46c46fe1ae9151e
shipit-source-id: 7afefd428a5fcb03867bcb69e46c46fe1ae9151e
Summary:When a component prop is set to null/undefined, and doesn't have a default value specified in `getDefaultProps`, the null value is sent over the bridge as a sentinel to reset to the original native value.
On iOS this is handled by creating a default view instance for each view type. The default view is then used to look up the unmodified value for any prop that is reset.
This is rather expensive however, as it means that for complex views (e.g. WebView, MapView), a minimum of two instances will be created even if only one is needed, and the default view will remain even after all actual view instances have been released.
This diff replaces the default view mechanism with a system where the default value of each prop is recorded the first time it is set. This avoids the need to keep an extra copy of the whole view.
The only exception is for props that use the `RCT_CUSTOM_VIEW_PROPERTY` macro, which includes the default view as part of the interface. To avoid a breaking change, a default view will still be created for views that use this macro, but only if they are sent a null value (so very rarely, in practice). In a future update we may deprecate or replace `RCT_CUSTOM_VIEW_PROPERTY` if there are significant benefits to doing so.
Reviewed By: javache
Differential Revision: D3012115
fb-gh-sync-id: 259348e54aa8342f444ad182b6f883d2dd684973
shipit-source-id: 259348e54aa8342f444ad182b6f883d2dd684973
Summary:Fixes the RefreshControl layout after a screen rotation. See #6311 for a more detailed explanation. I fixed it by adjusting the frame of the RefreshControl in `layoutSubviews` of the parent ScrollView.
While working on fixing this I noticed that when doing a 'pull to refresh' and then not scrolling and wait for it to end the next one will not behave like the first one (it will require pulling further down for the spinner to start spinning). I fixed that too by scrolling the scrollview back to 0 manually before calling `UIRefreshControl.endRefreshing`.
**Test plan (required)**
Tested using the UIExplorer RefreshControl example.
When doing a pull to refresh and then rotating the screen the RefreshControl must stay positioned properly.
Doing multiple consecutive pull to refresh without scrolling after should all behave the same.
Fixes#6311
Closes https://github.com/facebook/react-native/pull/6359
Differential Revision: D3023727
fb-gh-sync-id: f50ae52ea769c2b3e5025c362544a8809a71aa00
shipit-source-id: f50ae52ea769c2b3e5025c362544a8809a71aa00
Summary:The bug is caused by a weird race condition. What happens is that when calling `UIRefreshControl#endRefreshing` the `UIScrollView` delegate `scrollViewDidScroll` function is called synchronously and then `dockClosestSectionHeader` crashes because the sticky header indexes are updated but not the contentView children.
I fixed it by adding an updating property on `RCTRefreshControl` and setting it before calling `endRefreshing` so we can know not to call `dockClosestSectionHeader` at that moment.
Tested with both `RefreshControl` and `onRefreshStart` prop.
I reproduced the bug by replacing ListViewExample.js in UIExplorer with https://gist.github.com/janicduplessis/05fc58e852f3e80e51b9Fixes#5440
cc nicklockwood
Closes https://github.com/facebook/react-native/pull/5445
Differential Revision: D2953984
Pulled By: nicklockwood
fb-gh-sync-id: c17a6a75ab31ef54d478706ed17a8115a11d734e
shipit-source-id: c17a6a75ab31ef54d478706ed17a8115a11d734e
Janic Duplessis
Valentin Shergin
Jake Murzy
Adam Miskiewicz
Saad Ismail
Tomas Reimers
Steffen Matthischke
Nathaniel Rankin Webb
John O'Leary
Feng999
Ben Alpert
Martin Konicek
Pieter De Baets
Martin Kralik
Mehdi Mulani
Douglas Lowder
Antti Moilanen
Skotch Vail
Clement Genzmer
Nick Lockwood
Gerald Monaco
Reem Helou
Adam Comella
Kyle Corbitt