Summary: For some reason the scroll events are sometimes generated with highly irregular spacing, some coming less than a millisecond apart. For interactions that must track scrolling exactly, this can cause them to glitch. With a scroll throttle of less than 17 ms, the intention is clear that the UI should be updated in sync with the scroll view so we shouldn't drop any events.
Reviewed By: PeteTheHeat
Differential Revision: D15068841
fbshipit-source-id: 730e7cb29cc3ddae66f37cf7392e02e0cc9d7844
Summary:
Per a conversation with TheSavior, in #24538, this adds snapshot tests for all components whose mocks will be addressed in that PR. Shallow and deep snapshots are included.
[General] [Added] - Snapshots
Pull Request resolved: https://github.com/facebook/react-native/pull/24554
Differential Revision: D15062197
Pulled By: cpojer
fbshipit-source-id: 70ddbaa5e6d1d2c0fd1130ab04c458d9c49d0ee8
Summary: adding the flow types makes it impossible to forget to change them back.
Reviewed By: yungsters
Differential Revision: D14990037
fbshipit-source-id: d018e4cf6798d50bcfb44b55d3c68ca7f5beef72
Summary:
By default, an alert is `cancelable` on Android but not on iOS.
This PR changes the behavior so that the Alert is not dismissable on Android by default.
The motivation is that many developers develop on iOS and test on Android, and do forget to consider the case that the alert is dismissable.
Consistent behavior by default makes it easier to develop cross-platform apps in general.
---
For context and for your consideration, I have started a discussion here with the topic of whether React Native should try to use OS defaults or be consistent between platforms:
https://github.com/react-native-community/discussions-and-proposals/issues/121
---
If this PR gets merged, the docs should be updated as well:
https://github.com/facebook/react-native-website/blob/master/docs/alert.md#android
[Android] [Changed] - By default, alerts are not dismissable
Pull Request resolved: https://github.com/facebook/react-native/pull/24541
Differential Revision: D15044798
Pulled By: cpojer
fbshipit-source-id: 762b2ace69eb7ec79cd6ebec916e473348b9cafe
Summary:
Fixes a regression in 1f8b46a2fc. The internal subscription vendor uses a sparse array to track listeners, which makes listener removal fast. When querying listeners, the sparse entries need to be removed. `Array#filter` is a built-in way to do this -> linked to the JS spec, which explains this.
[General] [Fixed] - Fixed sparse array handling in `EventEmitter#listeners()`
Pull Request resolved: https://github.com/facebook/react-native/pull/24546
Differential Revision: D15044790
Pulled By: cpojer
fbshipit-source-id: 0f1301618739357b4a0f5378b9584efe74f0f09a
Summary:
Makes sure `onViewableItemsChanged` fires ASAP when `waitForInterations` is false.
This also works around another deeper bug where updates scheduled with `InteractionManager` aren't firing at all in some cases, and thus instead of just firing late, `onViewableItemsChanged` isn't firing until scroll which is not what we want with `waitForInterations: false`. That bug will require more digging.
Differential Revision: D14984333
fbshipit-source-id: 718b39670307c6bc16268759bdb513682745265d
Summary:
This diff fixes the reconnect logic with the metro websockets which is causing the app to not re-connect when metro crashes. To demonstrate the issue, consider the following video:
{F156029086}
On the left we have metro, on the right is the xcode console with some logging to show the reconnecting phase. When we kill the metro server you can see the app tries to reconnect once and that's it - when metro is started back up, you can see the notification that there are no apps running and can also see that cmd+opt+r doesn't work anymore
I updated the logic to optimistically start the connection and if it's still unavailable to retry again after the timeout
[iOS][Fixed] - Metro websocket reconnect logic
Reviewed By: shergin
Differential Revision: D14961433
fbshipit-source-id: 0569aa169dc9f538a7e4a8d04e99de39f2e9b3f9
Summary:
In order to meet our accessibility requirements we need to have full support for keyboard navigation. The Touchable components works with press/tap with a finger, but doesn't respond to 'enter' when using a keyboard. Navigation works fine. This PR adds an onClick listener to touchable views that have the onPress prop defined.
[Android] [Added] - Add View.OnClickListener to Touchable components when onPress is defined
Pull Request resolved: https://github.com/facebook/react-native/pull/24359
Differential Revision: D14971230
Pulled By: cpojer
fbshipit-source-id: ca5559ca1308ee6c338532a00dcea4d00fa57f42
Summary:
Second attempt at adding border properties to `RCTImageView`.
Previous attempt can be found at D14875673 which was reverted.
1. `UIImageView` is no longer laid out in in `layoutSubviews`.
2. `updateWithImage` was not being called, I instead of calling `self.image = image` I was calling `_imageView.image = image` directly which skips calling `updateWithImage`. This meant that image's rendering mode was not changed to `template`.
Reviewed By: shergin
Differential Revision: D14934103
fbshipit-source-id: b74c692f9f8ad520ef1f9c70ec4b4aa68b868cd4
Summary:
Would like feedback from the community as this may not be the best solution for all
I would like to restrict (or paginate) the fling of a horizontal ScrollView when `snapToInterval` is set. This is not currently possible with `pagingEnabled`, since the pagination works only when items are the entire width of the ScrollView.
This implementation simply restricts the predicted `targetOffset` found from the `x` velocity and replaces it with the offset when the pan gesture ended.
To get pagination working, I may paginate based on the interval by calculating the offset delta from the beginning of the gesture to current offset and restricting the scrolling behavior to the `snapToInterval`. If this is preferred, I can update this PR or make a new one, but wanted to start a discussion since it seems like there are many in the community that would like this feature #21302 .
[General] [Added] - add prop `disableIntervalMomentum` to disable the predictive scrolling behavior of horizontal ScrollViews
Pull Request resolved: https://github.com/facebook/react-native/pull/24045
Differential Revision: D14939754
Pulled By: sahrens
fbshipit-source-id: 26be19c47dfb8eed4d7e6035df53a77451e23081
Summary:
This showcases SampleTurboModule usage in RNTester. Notes:
* iOS only for now, and you must use cocoapods version.
* You cannot use Chrome debugger when loading this specific example.
As illustrated in the example, the callsite should access `NativeSampleTurboModule` to access the native side.
{F155901711}
Reviewed By: cpojer
Differential Revision: D14932537
fbshipit-source-id: a733e1cd3b642b9e572d5ac6347f4775d495578a
Summary:
This provides various versions of SampleTurboModule, that are:
* compatible with existing NativeModule
* TurboModule compliant
Variants:
* RCTSampleTurboModule (traditional objc module)
* RCTSampleTurboCxxModule (objc++ module using CxxModule)
* SampleTurboModule (pure C++ impl of a TurboModule, no ObjC)
As noted in some files, they need to be codegen'ed based on the `NativeSampleTurboModule.js` (Flow type). The codegen script is not yet usable in OSS (we'll work on it some time in H2 2019). For now, these files need to be manually synced with Flow type.
Reviewed By: cpojer
Differential Revision: D14932539
fbshipit-source-id: fb887192384e5e6e4dff4cac68b4e037a4783cd9
Summary:
This adds https://github.com/mysticatea/abort-controller to polyfill [AbortController](https://developer.mozilla.org/en-US/docs/Web/API/AbortController). This is used to cancel requests when using `fetch`.
This also updates `event-target-shim` to 5.0 to make sure we only have one version of this dependency. This updates required adding a polyfill for `console.assert` which is used by the new version. I made one based on https://github.com/gskinner/console-polyfill/blob/master/console.js#L74.
The polyfill is very small, especially since we already use `event-target-shim` so I think it makes sense to include in core.
Depends on #24418 so that the fetch polyfill supports the `signal` parameter.
Fixes#18115
[General] [Added] - Add support for cancelling fetch requests with AbortController
Pull Request resolved: https://github.com/facebook/react-native/pull/24419
Differential Revision: D14912858
Pulled By: cpojer
fbshipit-source-id: 8a6402910398db51e2f3e3262f07aabdf68fcf72
Summary:
Closes: https://github.com/facebook/react-native/issues/24016
React Native 0.57 introduced cross-platform `accessibilityRole` and `accessibilityStates` props in order to replace `accessibilityComponentType` (for android) and `accessibilityTraits` (for iOS). With #24095 `accessibilityRole` and `accessibilityStates` will increase, receiving more options, which seems to be a good moment to remove deprecated props.
Remove deprecated `accessibilityComponentType` and `accessibilityTraits` props.
[General] [Removed] - Remove accessibilityComponentType and accessibilityTraits props
Pull Request resolved: https://github.com/facebook/react-native/pull/24344
Reviewed By: rickhanlonii
Differential Revision: D14842214
Pulled By: cpojer
fbshipit-source-id: 279945e503d8a23bfee7a49d42f5db490c5f6069
Summary:
`setupDevtools.js` is accessing `AppState.currentState` without checking its availability. In environments where 1) `__DEV__ == true`, and 2) no `RCTAppState` native module is provided thus resorting to `MissingNativeAppStateShim`, this will result in an exception:
```Cannot use 'AppState' module when native 'RCTAppState' is not included in the build. Either include it, or check 'AppState'.isAvailable before calling any methods.```
(Interestingly, `MissingNativeAppStateShim.currentState` did have a [default `null` value](118e88393e (diff-305b5180aa6ccc876ede6767de1fbfc4R192)) that was [later removed](a93b7a2da0 (diff-305b5180aa6ccc876ede6767de1fbfc4R186)).)
**Update**: Following cpojer's suggestion of reverting a93b7a2da0. Title also updated to reflect this.
[General] [Fixed] - Remove MissingNativeRCTNetworkingShim; revert MissingNativeAppStateShim
Pull Request resolved: https://github.com/facebook/react-native/pull/24380
Differential Revision: D14932658
Pulled By: cpojer
fbshipit-source-id: aef7ca566b3b8660eaed74a8ba3b6b0117b1200c
Summary:
This PR fixes#24229.
Seems currently `opacity` props for Text is being applied twice (one for text color and one for the whole view). This PR disables applying the prop to the text.
[CATEGORY] [TYPE] - Fixed double applying opacity prop for Text
Pull Request resolved: https://github.com/facebook/react-native/pull/24435
Differential Revision: D14932795
Pulled By: cpojer
fbshipit-source-id: f9280fc75f788424cb5f1e42d2e79efdb354d645
Summary:
`RCTImageView` is now a subclass of `RCTView` and includes `UIImageView` as it's subview.
This enables the use of `borderRadius`, `borderWidth`, `borderColor` properties and all of their derivatives.
Possible problem:
Now `RCTImageView` is backed by two views (`RCTView` + `UIImageView`), not a single one. That's 4 `CALayers`. Possible workaround would be to insert the image directly into `self.layer.contents`.
Reviewed By: RSNara
Differential Revision: D14875673
fbshipit-source-id: 594b2cd1ddffc6627566e07983c6d8f0b37dc2bb
Summary:
There is a problem rendering text shadows on iOS. If the offset of the text shadow is `{width:0,height:0}`, the shadow does not display. This prevents you from representing a light directly above the text. This occurs because a text shadow only renders if the offset is a non-zero CGRect `{width:0,height:0}`.
My change checks `textShadowRadius` instead. If `textShadowRadius` is not nan then the user is rendering a text shadow. There are no situations to render a shadow without `textShadowRadius` making it a good variable to check.
This PR fixes this stale issue: https://github.com/facebook/react-native/issues/17277
[iOS] [Fixed] - Text shadow now displays when the textShadowOffset is {width:0,height:0}
Pull Request resolved: https://github.com/facebook/react-native/pull/24398
Differential Revision: D14890768
Pulled By: cpojer
fbshipit-source-id: a43b96a4a04a5603eede466abacd95c010d053e5
Summary: AppRegistry always hides the small fabric indicator. This allows it to be enabled.
Reviewed By: mdvacca
Differential Revision: D14877831
fbshipit-source-id: 4fe3aac13074641049a9298f5a616aea3138ce93
Summary:
This allows an unsupported component to be rendered as a "unimplemented view" for better visualization of which component is missing. It is off by default, but configurable in the component factory.
For now, the layout simply follows regular <View />, which means the width/height etc is based on the react component styling. The side effect is that components with 0 height/width won't show up at all.
Reviewed By: mdvacca
Differential Revision: D14869656
fbshipit-source-id: f31e012fb7dc1c64fcc431ea5aa45079a23a618e
Summary:
Changelog:
----------
[Changed][General] Move callback-related logic to `AnimatedNode` class in order to make it possible to add the listener for other animated nodes than `AnimatedValue`.
I observed that native code appears to be fully prepared for listening not only to animated value but animated nodes generally. Therefore I managed to modify js code for exposing `addListener` method from `AnimatedNode` class instead of `AnimatedValue`. It called for some minor changes, which are not breaking.
If you're fine with these changes, I could add proper docs if needed.
Pull Request resolved: https://github.com/facebook/react-native/pull/22883
Differential Revision: D14041747
Pulled By: cpojer
fbshipit-source-id: 94c68024ceaa259d9bb145bf4b3107af0b15db88
Summary:
Flow typing can be annoying because the `renderItem` prop for FlatList always has to specifically be of type `React.Element`.
Since really we just want to return something renderable, it should be fine to type this as `React.Node` instead.
I'm not sure if this is valid, but it seems like since we just need to implant the `key` property, we should be able to accomplish this with a `React.Fragment` wrapper instead of needing to call `cloneElement`. Looking for feedback on if this is a sensible fix.
Changelog:
[General][Changed] Updated FlatList's renderItem Flow type from React.Element<any> to React.Node
Reviewed By: sahrens
Differential Revision: D14814805
fbshipit-source-id: ce6793dea5a92619babe048dcfddee0e4519979c
Summary:
This pull request fixes#24257.
The wrapper around ListEmptyComponent doesn't allow to use flex on the ListEmptyComponent.
The wrapper was removed in this [commit](db061ea8c7), and then put back in this [commit](e94d3444dc) but I think the relevant part was removing the condition on `itemCount !== 0` to apply the inversionStyle on the ScrollView and everything is still working without the wrapper.
[GENERAL] [FIXED] - Remove wrapper around ListEmptyComponent
Pull Request resolved: https://github.com/facebook/react-native/pull/24339
Differential Revision: D14822221
Pulled By: cpojer
fbshipit-source-id: 623e1ab3f228e9b75b92cdcd568683232a403c1a
Summary:
Main change is to the property diffing - we now use the last known props set on the view rather than the default props to compute the diff. This requires exposing a `getProps` method on all view components which should be fine I think.
I also realized that in more complex animations with multiple nodes, the node that the animation starts on might not be connected to a view, so we don't know if it's fabric just based on that, so we have to do a recursive search through the children to find if there are any that are associated with a fabric view to decide we should start the animation immediately. Unfortunately there can still be a timing gap here since the animated API is async and the uimanager API is sync - I'll need to change the animated API to be sync to completely fix this.
Reviewed By: shergin
Differential Revision: D14732028
fbshipit-source-id: 882c056b0b63aa576f8e42439be405cf7fb3147a
Summary:
We need to move animated native module calls to synchronous so we can properly thread them in with mounting instructions in Fabric. Some of them take callbacks so we need to add support for that when switching to synchronous.
A side benefit is that we can unify codepaths a little more with async callbacks.
Reviewed By: shergin
Differential Revision: D14790898
fbshipit-source-id: dc222b9e74375e046e8a9b1b19d72f652dc6722c
Summary:
Currently calling native methods on internal react native components throw a warning. I believe this is problematic because _users_ aren't calling native methods on internal components, the _component_ is making the call.
So for instance, if I unmount a component that has a form with a few uses of `TextInput`, which is a perfectly valid test case, my test output will be full of warnings that I can't call `.blur()` in the test renderer environment. That's very misleading, because I didn't, the internal component did. In fact, as far as I can tell, there's not really even anything I can do to stop that call or use the output from it, its all internal. `TextInput` is a black box, and 99% of users writing tests probably won't even know it calls `.blur()` under the hood on unmount.
I want to change these to `jest.fn()` because I think this eliminates a lot of chatter in test output, but also doesn't send users down a rabbit hole of trying to find workarounds that may involve filtering console output, which could potentially lead them to inadvertently filter out real warnings that they should see.
So I'm willing to change the implementation of how I did this, but I don't think its right to warn users that they called a native method when they didn't. If they build a component that calls these methods, I believe it's on them to do something similar to this, and maybe we can make this exposed as a helper that can be used for third party component mocks?
[General] [Changes] - Changed MockNativeMethods for core components to `jest.fn()` instead of function that warns about calling native methods.
Pull Request resolved: https://github.com/facebook/react-native/pull/24337
Differential Revision: D14822126
Pulled By: cpojer
fbshipit-source-id: 2199b8c8da8e289d38823bdcd2c43c82f3f635c9
Summary:
Setting the scroll throttle for every animated scrollview is a pain, and if you forget it's super janky and can be confusing and frustrating.
Enables setting default props in createAnimatedComponent and uses it for scrollview.
Reviewed By: TheSavior
Differential Revision: D14790093
fbshipit-source-id: dd8f6f6540813245e87d696351f09ebb2e6ed5f2
Summary:
Fix#23755 - Which is to remove the warning:
```
Module RCTImagePickerManager requires main queue setup since it overrides `init` but doesn't implement `requiresMainQueueSetup`. In a future release React Native will default to initializing all native modules on a background thread unless explicitly opted-out of.
```
General Fixed - Warning "RCTImagePickerManager requires main queue setup
Pull Request resolved: https://github.com/facebook/react-native/pull/24314
Differential Revision: D14788772
Pulled By: cpojer
fbshipit-source-id: e2017136008367d36468debb258afa698b5402bc
Summary:
We don't need these polyfills in RN Open Source any longer because JSC supports these features natively.
We also don't need these internally at FB, but I want the removal to be a step by step process and separate from the open source work.
Reviewed By: rickhanlonii
Differential Revision: D14762827
fbshipit-source-id: 114626bd18516c42ced43c3f7aa29d42d1d95335
Summary:
It seems (I used git history to confirm) that FlatList/VirtualizedList have ([since the begining](c13f5d48cf/Libraries/Lists/VirtualizedList.js (L79))) a `disableVirtualization` prop.
SectionList ([since it's begining](abe737fe74/Libraries/Lists/VirtualizedSectionList.js (L98))) have a `enableVirtualization` prop, but since SectionList is VirtualizedSectionList which use VirtualizedList, this prop probably never did something. This fix just rename the prop properly so it can have an effect on the underlying VirtualizedList when you use a SectionList.
Since props are spread it's kind of working already, but the flow annotation are wrong (so it tells you it won't work/ you can't use it) which sucks.
(NB: I am doing this since I was trying to use a SectionList with react-native-web & server side rendering to get the all list, you can laugh).
[General] [Fixed] - VirtualizedSectionList/SectionList: replace enableVirtualization prop annotation by correct underlying disableVirtualisation of VirtualizedList
Pull Request resolved: https://github.com/facebook/react-native/pull/24312
Differential Revision: D14779449
Pulled By: cpojer
fbshipit-source-id: e51e1d639d2bb265b5b286786010d01ffd9d90e0
Summary: These are the same types as the existing measure calls on Paper's UIManager except ReactTag has been replaced with Node
Reviewed By: fkgozali
Differential Revision: D14732142
fbshipit-source-id: b757e0d1f8d168232d8ba58938cdcd3b30e006ff
Summary: Now that React Native ships with a newer version of JSC, we do not need this code to wrap `Object.freeze` any longer.
Reviewed By: mmmulani
Differential Revision: D14779239
fbshipit-source-id: 1a6e1a9c7f4312572bd08ba604fa8c9d6b1927e1
Summary: We've identified a couple of remaining issues that need to be re-tested before we can ship this more broadly.
Reviewed By: fred2028
Differential Revision: D14775730
fbshipit-source-id: 22402149066c5fbe72c36fcf7f547d63feaf5241
Summary:
ScrollView's scrollTo behavior on iOS was recently changed to limit the offset to the content size plus any content inset (see #23427). This departure from the old behavior created UI issues for anyone that is using the over-scroll ability for the purpose of positioning elements at specific coordinates on the screen. Examples include using this behavior to position TextInputs above the virtual keyboard programmatically when focused or moving drop down elements positioned near the bottom of the content toward the top of the screen when selected to show a larger absolutely positioned item list. Default behavior does not change and this is an "opt-in" type of prop to re-enable the old behavior.
[iOS] [Added] - Added scrollToOverflowEnabled prop to ScrollView
Pull Request resolved: https://github.com/facebook/react-native/pull/24296
Differential Revision: D14762619
Pulled By: cpojer
fbshipit-source-id: d2a552b5cb321d52e8ea4116327bf9ec647a3aae
Summary:
This diff fixes a bug in TouchableNativeFeedback where a long press is not registered.
cause of the bug is _touchableHandleResponderMove_ being invoked **regardless** of a moving gesture ( even when movedDistance is 0) in some devices ( including OnePlus5t ), which was eventually clearing out the long-press timeout.
fix is to prevent _touchableHandleResponderMove_ from Implementing if the state of touchable is RESPONDER_INACTIVE_PRESS_IN.
[General] [Fixed] - Touchable onLongPress fix.
Pull Request resolved: https://github.com/facebook/react-native/pull/24238
Reviewed By: cpojer
Differential Revision: D14712986
Pulled By: rickhanlonii
fbshipit-source-id: e85a66a7e8b61e0a33146b2472e2e055726a0e93
Summary: All animations are scheduled by the UIManager while it processes a batch of changes, so we can just wait to see what the longest animation is and cancel+reschedule the callback.
Reviewed By: mdvacca
Differential Revision: D14656733
fbshipit-source-id: 4cbbb7e741219cd43f511f2ce750c53c30e2b2ca
Summary:
@public
This resolves the iOS side of #20879.
Reviewed By: natestedman, cpojer
Differential Revision: D14712066
fbshipit-source-id: 88dd0ff80d3467b314cacb9349029dadca4ddf19
Summary:
Potential breaking change: The signature of ReactShadowNode's onBeforeLayout method was changed
- Before: public void onBeforeLayout()
- After: public void onBeforeLayout(NativeViewHierarchyOptimizer nativeViewHierarchyOptimizer)
Implements same feature as this iOS PR: https://github.com/facebook/react-native/pull/7304
Previously, only Text and Image could be nested within Text. Now, any view can be nested within Text. One restriction of this feature is that developers must give inline views a width and a height via the style prop.
Previously, inline Images were supported via FrescoBasedReactTextInlineImageSpan. To get support for nesting views within Text, we create one special kind of span per inline view. This span is called TextInlineViewPlaceholderSpan. It is the same size as the inline view. Its job is just to occupy space -- it doesn't render any visual. After the text is rendered, we query the Android Layout object associated with the TextView to find out where it has positioned each TextInlineViewPlaceholderSpan. We then position the views to be at those locations.
One tricky aspect of the implementation is that the Text component needs to be able to render native children (the inline views) but the Android TextView cannot have children. This is solved by having the native parent of the ReactTextView also host the inline views. Implementation-wise, this was accomplished by extending the NativeViewHierarchyOptimizer to handle this case. The optimizer now handles these cases:
- Node is not in the native tree. An ancestor must host its children.
- Node is in the native tree and it can host its own children.
- (new) Node is in the native tree but it cannot host its own children. An ancestor must host both this node and its children.
I added the `onInlineViewLayout` event which is useful for writing tests for verifying that the inline views are positioned properly.
Limitation: Clipping
----------
If Text's height/width is small such that an inline view doesn't completely fit, the inline view may still be fully visible due to hoisting (the inline view isn't actually parented to the Text which has the limited size. It is parented to an ancestor which may have a different clipping rectangle.). Prior to this change, layout-only views had a similar limitation.
Pull Request resolved: https://github.com/facebook/react-native/pull/23195
Differential Revision: D14014668
Pulled By: shergin
fbshipit-source-id: d46130f3d19cc83ac7ddf423adcc9e23988245d3
Summary: This removes the JS parts of Geolocation from React Native open source.
Reviewed By: yungsters
Differential Revision: D14693179
fbshipit-source-id: 1da5b7ec0e3e9d21d2019b7ee43e5f85661795b4
Summary: This is the first diff in an effort to remove Geolocation from React Native. This diff removes the globally injected navigator.geolocation feature and instead requires explicit importing of `Geolocation`. When using Web APIs, people will need to patch `navigator.geolocation` on their own from now on.
Reviewed By: sahrens
Differential Revision: D14692386
fbshipit-source-id: c57b290b49728101250d726d67b1956ff23a9a92
