Summary: We should not use `self.contentView` accessor to insert container views (views that might be used as a container for nested views) into component view otherwise it causes double applying of padding values.
Reviewed By: mdvacca
Differential Revision: D15862075
fbshipit-source-id: aad834aeca06f66b19c2f7bbaa0b58f937e66994
Summary: Setting value directly to ivar makes impossible to subclasses to react of this; this is a bug.
Reviewed By: mdvacca
Differential Revision: D15776691
fbshipit-source-id: 10f9be975ee673b2db46c24ba41b18f6d6ddf0a3
Summary:
In some cases, the view class is the only that retains stored `props_` variable. At the same time the `[super updateProps:props oldProps:oldProps];` actually resets the pointer with a pointer to new props value which sometimes causes the deallocation of the old value. All that is okay unless the implementation of `updateProps:oldProps:` in superclasses stores a raw reference to an old value in the very beginning of the method (for convenience and perf reasons).
So, to prevent preliminary deallocation of the old value pointed by `_props` we moved all `[super updateProps:props oldProps:oldProps];` calls to the end of overloaded methods.
Reviewed By: mdvacca
Differential Revision: D15770068
fbshipit-source-id: af36b3e70560ab00846cd26b0963bbc059e977bc
Summary: It's a antient left over which was copy pasted a dozen of times. All `RCTViewComponentView` subclasses have own copy of the old props `_props`, so it's useless to check for `oldProps`.
Reviewed By: mdvacca
Differential Revision: D15770067
fbshipit-source-id: 39f4d71ccdcf0c9a5b911b17a3b922dbe6dd9a8e
Summary: This makes it so that you don't have to specify the package to the bundle. When you don't specify it, it will use the default bundle that is chosen when the app starts.
Reviewed By: PeteTheHeat
Differential Revision: D15742001
fbshipit-source-id: cd5906a66d04eea308dbc0516bc1fec60caed6bb
Summary:
The necessity of this feature was removed in 2017. To intercept React Native gesture recognizer, implement UIGestureRecognizer delegate for conflicting gestures.
Here is the quote from the internal note:
> Previously we had lots of super weird bugs where React Native would inaccurately recognize touch gestures which were meant to be addressed by the native environment. Usually, these bugs occurred as unintentional taps happening right after swipe gestures. In all of these cases, we had to manually call method `cancelTouches` as part of an external gesture recognition process which prevented touch delivery to React Native. Furthermore, we had to delay touch delivery to React Native to wait for these cancellations. That code always looked like a hack (in the bad meaning of this word), like in some random place something dispatch event to another random place where something finally calls `cancelTouches`, yak. It was super annoying because it required adding this hack to all existing apps and screens, and because sometimes it was even too late to cancel touches.
> We fixed that. Instead of delaying touch delivery and waiting for calls to `cancelTouches`, we set up the React Native gesture recognizer in such way that it always agrees to fail in favor of any native gestures (from non-RN-based and served views which are placed higher in a hierarchy). React Native will now cancel all active touches itself so that we no longer need to call `cancelTouches` manually. We already removed all these calls and supported code from Facebook and Instagram.
See also: https://github.com/facebook/react-native/pull/25193
Reviewed By: PeteTheHeat
Differential Revision: D15734129
fbshipit-source-id: 289f77a437cb40199c591153b5801d24d0c10d1e
Summary: We have to use a getter instead of an ivar to enable lazy initialization of ContextContainer.
Reviewed By: mdvacca
Differential Revision: D15731877
fbshipit-source-id: eb4d0e70c337026a91cb12a3eb26ed4d94f39f9f
Summary:
... and slighly new behaviour for one of them.
The method does nothing if given `key` already exists in the container.
This diff finishes the transition of ContextContainer from an internal bag of things with unclear yet ownership into a legit dedicated dependency injection container for the product code.
The original names of methods imply that the container can have only one object of a given type which is no longer true. The new API is much more generic and idiomatic to C++, it mimics `std:map` API which is intuitive to anyone who familiar with C++ containers.
Besides the naming, `insert` method changed the semantic a bit; now it does nothing in case of inserting an object with a key that already exists. That might seem counterintuitive for "normal" people, but C++ has some wired reasons for that and, hopefully, it's expected behavior in the C++ community.
Fun fact: We need this to fix hot-reload.
Reviewed By: sahrens
Differential Revision: D15681736
fbshipit-source-id: 194f342528446a911eaf072ba3a94a5d8af3cb52
Summary: Restores the bridge description in the debug menu on iOS.
Reviewed By: fkgozali
Differential Revision: D15680775
fbshipit-source-id: c17ad44f2287e03bb2039b4aa4b1311e7ec9106b
Summary:
In environments with McAfee EPro software, port 8081 is used and cannot be unbound. (See [#20466](https://github.com/facebook/react-native/issues/20466) for a recent example issue.) Most issues can be worked around by passing a `--port` option or setting `RCT_METRO_PORT`, but there are a couple places where 8081 is hardcoded that block adoption. Right now the workaround I've seen pushed on Stack Overflow and elsewhere is to modify node_modules after a `yarn install`, so I'd like to fix it at the source.
This PR changes the react "Start Packager" build step and some code in the iOS DevSupport library to read from the `RCT_METRO_PORT` variable.
## Changelog
[General] [Changed] - Removed hardcoded references to port 8081 and replaced them by reading from RCT_METRO_PORT (w/ fallback to 8081)
Pull Request resolved: https://github.com/facebook/react-native/pull/25144
Differential Revision: D15630119
Pulled By: cpojer
fbshipit-source-id: aff5d24691e0e9892a035cfbd25330fed6441199
Summary:
Fixes build in Xcode 11 beta, the signature for `__unused` was changed. This adds a new check for the new style.
## Changelog
[iOS] [Fixed] - Xcode 11 beta build
Pull Request resolved: https://github.com/facebook/react-native/pull/25146
Differential Revision: D15628404
Pulled By: cpojer
fbshipit-source-id: 781a188a0e1562a3316fbe62920b12b03a44e4a7
Summary:
The accessibilityActions accessors in UIView+React.m are not aligned with the property declaration in the header file.
## Changelog
[General] [Fixed] - Fix accessibilityActions accessors
Pull Request resolved: https://github.com/facebook/react-native/pull/25134
Differential Revision: D15621848
Pulled By: cpojer
fbshipit-source-id: f344689292ae7988e46d0d4263980306d364366b
Summary:
I noticed that the RNTester-tvOS target is not compilable when I wanted to test the TVOS capacity of React Native.
More specifically, the changes included in this PR are:
### RNTester-tvOS target
1. Add `AppDelegate.mm` to the target.
2. Add `.m` files under `turbomodule` to the target.
3. Add the following directories to **header search path**.
```
$(SRCROOT)/../third-party/boost_1_63_0
$(SRCROOT)/../third-party/folly-2018.10.22.00
$(SRCROOT)/../third-party/glog-0.3.5/src
```
4. Add `RN_BUNDLE_PREFIX` to the scheme argument.
5. Add `RN_BUNDLE_PREFIX` entry to the plist file.
### React-tvOS target
1. Add `RCTCxxBridgeDelegate.h` and `JSCExecutorFactory.h` to the **Copy headers**.
## Changelog
[iOS] [Fixed] - Fixed the issue that the RNTester-tvOS is not compilable.
Pull Request resolved: https://github.com/facebook/react-native/pull/25110
Differential Revision: D15602450
Pulled By: cpojer
fbshipit-source-id: e7eda18c8193b7d88355feafa69043ffef4a8edb
Summary:
We need to use second for calculation, so change 17ms to 0.017s instead.
## Changelog
[iOS] [Fixed] - Fixes wrong time unit of scroll event throttle
Pull Request resolved: https://github.com/facebook/react-native/pull/25098
Reviewed By: sahrens, cpojer
Differential Revision: D15576526
Pulled By: sammy-SC
fbshipit-source-id: ddd8dd9098cbe582c6923ce8466892c363c090fc
Summary:
This PR fixes incorrect unhooking for UI manager views in `RCTProfileUnhookModules`. `view` is actually a key, not the view itself; instead, use `viewForReactTag:` to obtain the view itself.
This fixes issue #24952.
## Changelog
[iOS] [Fixed] - Fix incorrect unhooking for UI manager views in `RCTProfileUnhookModules`, causing an infinite `RCTProfileTrampoline ` recursion (#24952)
Pull Request resolved: https://github.com/facebook/react-native/pull/25042
Differential Revision: D15580978
Pulled By: PeteTheHeat
fbshipit-source-id: 3483a7f6380b6fb1db4249374d86f692348c9aa2
Summary:
Addresses a number of pieces of feedback regarding the debug menu.
- Simplify labels for the debugger actions (e.g. no "remote", no emoji).
- Reorder actions so that modal items are generally lower.
- Changed "Toggle Inspector" to "Show/Hide Inspector".
- Renamed "Live Reloading" to "Reload-on-Save".
- Hide disabled debug items when profiling is enabled.
- Changed "Start Systrace" to "Systrace Unavailable" when debugging.
- Renamed "Change packager location" to "Configure Bundler".
- Revised nomenclature in "Configure Bundler" menu to be clearer.
- Removed extraneous debug menu title.
- Consistently refer to HMR as "Hot Reloading".
Changelog:
[iOS] [Changed] - Cleaned up debug menu.
Reviewed By: axe-fb
Differential Revision: D15548628
fbshipit-source-id: 26b2ddca8280d1f6f8ff904439b403600e98a3b3
Summary:
Related #23674, in that PR, we imported background timer support, but it's not sufficient, I think the reason that works is because it enable the `Background Modes` and do some background tasks, for the users who don't enable it, timer would pause immediately before goes into background.
To fix it, we can mark a background task when goes into background, it can keep app active for minutes, try best to support timing when in background.
cc. cpojer .
## Changelog
[iOS] [Fixed] - Timing: Fixes timer when app get into background
Pull Request resolved: https://github.com/facebook/react-native/pull/24649
Differential Revision: D15554451
Pulled By: cpojer
fbshipit-source-id: a33f7afe6b63d1a4fefcb7098459aee0c09145da
Summary: We need this only until Fabric has own command-execution pipeline.
Reviewed By: mdvacca
Differential Revision: D15501202
fbshipit-source-id: aad77660ada43e429722b13d1da2f998a1726c73
Summary: This is a temporary workaround that we need only temporary and will get rid of soon.
Reviewed By: mdvacca
Differential Revision: D15501203
fbshipit-source-id: cec4891b6a185ea9e39f50bfedf9e1dae8993b66
Summary:
As PR [#24743](https://github.com/facebook/react-native/pull/24743) suggested, accessibility properties are better to set on UIView+React instead of RCTView so they can be used safely on any UIView.
## Changelog
[General] [Fixed] - Move accessibilityActions props to UIView+React
Pull Request resolved: https://github.com/facebook/react-native/pull/25015
Differential Revision: D15503131
Pulled By: cpojer
fbshipit-source-id: dedf9afbd0a1d35a5abbd13ace95ee620f718f39
Summary:
Same as previous one but for the rest (minority) of methods.
I didn't change `updateLocalData:` because it's going away soon anyway.
Reviewed By: mdvacca
Differential Revision: D15473217
fbshipit-source-id: 6a6bd66c5343211a973fc34ad11e86efe031d07d
Summary:
Passing shared pointers as references can save us a couple of milliseconds at scale.
Originally, I didn't expect that Objective-C supports passing values by references, but apparently it does.
Reviewed By: mdvacca
Differential Revision: D15473218
fbshipit-source-id: 15eb3770cc0889654647a8e91607d8aa78010121
Summary:
As currently defined, accessibilityStates is an array of strings, which represents the state of an object. The array of strings notion doesn't well encapsulate how various states are related, nor enforce any level of correctness.
This PR converts accessibilityStates to an object with a specific definition. So, rather than:
<View
...
accessibilityStates={['unchecked']}>
We have:
<View
accessibilityStates={{'checked': false}}>
And specifically define the checked state to either take a boolean or the "mixed" string (to represent mixed checkboxes).
We feel this API is easier to understand an implement, and provides better semantic definition of the states themselves, and how states are related to one another.
## Changelog
[general] [change] - Convert accessibilityStates to an object instead of an array of strings.
Pull Request resolved: https://github.com/facebook/react-native/pull/24608
Differential Revision: D15467980
Pulled By: cpojer
fbshipit-source-id: f0414c0ef6add3f10f7f551d323d82d978754278
Summary:
UIView+ComponentViewProtocol is useless without it. Conformance to the protocol is only purpose of this category.
We didn't run into this issue because we have never used that yet.
Reviewed By: mdvacca
Differential Revision: D15419953
fbshipit-source-id: 84a430397e886c941d35075d9754eae5748c3a3f
Summary:
Updates the paragraph component to use State instead of Local Data, part of the path to a Fabric TextInput 💯
## Changelog
[General] [Changed] - Fabric: Use State instead of Local Data for Paragraph
Pull Request resolved: https://github.com/facebook/react-native/pull/24873
Differential Revision: D15410979
Pulled By: shergin
fbshipit-source-id: 3c9517d2495a64c4dbd213b6efb5ff55287900e3
Summary:
In case `RCTView` had no subviews, `RCTRecursiveAccessibilityLabel` was returning an empty string rather than nil.
I've noticed that https://fburl.com/i95iynxi returns empty string as well. Shall we change it there as well?
Reviewed By: shergin
Differential Revision: D15337865
fbshipit-source-id: 8e38a88321f3dcfdf35a38d43fdbe4f5118796f1
Summary:
This is a reconstitution of #24190. It extends accessibility actions to include both a name and user facing label. These extensions support both standard and custom actions.
We've also added actions support on Android, and added examples to RNTester showing how both standard and custom accessibility actions are used.
## Changelog
[general] [changed] - Enhanced accessibility actions support
Pull Request resolved: https://github.com/facebook/react-native/pull/24695
Differential Revision: D15391408
Pulled By: cpojer
fbshipit-source-id: 5ed48004d46d9887da53baea7fdcd0e7e15c5739
Summary:
Straightforward.
Rick, I rename some stuff, I hope you are cool with that.
Reviewed By: mdvacca
Differential Revision: D15403306
fbshipit-source-id: 1dbd34060052a9bd39ed4211010f14b76fffcde6
Summary: This is implementation of standard PullToRefresh component that uses standard iOS component and modern integration approach.
Reviewed By: mdvacca
Differential Revision: D15403308
fbshipit-source-id: 5c877f7c18af9f5ac40e15a4ba44118614ba80bc
Summary: This way we can reuse this code in other RCTScrollViewComponentView-satellite components, especially in standard pull-to-refresh component.
Reviewed By: mdvacca
Differential Revision: D15403307
fbshipit-source-id: 5999f851f22db0f358887e1a86d610e163adcb1d
Summary:
Fix Issue https://github.com/facebook/react-native/issues/24820
It's caused by `_labelsForTags` and `RCTPLTag` being out of sync, the crash might only be one of the issues that this bug was causing.
## Changelog
[iOS] [Fixed] - fix crash on performance logger
Pull Request resolved: https://github.com/facebook/react-native/pull/24821
Differential Revision: D15407291
Pulled By: PeteTheHeat
fbshipit-source-id: c8d2a047fceb9cec981c48fe5181d1b4cbf0976c
Summary:
This diff contains two changes:
* The actual UIScrollView is now mounted inside the component as `contentView` which mostly means that border-props will properly affect the layout of the scroll view (the scroll view will be laid out inside borders, not on top of those). And that also simplifies the code.
* Now the component view exposes the actual scroll view, its delegate splitter, and the container view defining a single interface for all possible integration that can be done with the Scroll View Component.
Reviewed By: mdvacca
Differential Revision: D15397283
fbshipit-source-id: 35e860b8bf55fbd4d0a5f4116f79e4507df79098
Summary:
There was a reason why it exists: on iOS if you want to implement a view with rounded corners with a non-transparent background you need to set `clipToBounds = YES` to enabling clipping because besides drawing borders we need to clip the background otherwise it will "stick out".
There were a bunch of problems with this implementation and approach:
* It's overshooting. It applies `clipToBounds` no matter which border-radius is and no matter is there a background or not. So, it's kinda heuristic implementation and it's totally unexpected behavior sometimes.
* The previous problems can lead to performance problems as well (clipping requires additional work to do for CoreGraphics/GPU).
* The border-radius that we assigned to contentView is incorrect because it does not take border-width into account.
* This functionality only applies to non-null `contentView` which is a rear and custom case in which that can always be done manually for components that require that for some reason.
Reviewed By: mdvacca
Differential Revision: D15397282
fbshipit-source-id: 1599882ca8e591a9c4edb4d2b845bbf3d147bfce
Summary:
## Overview
This diff switches the RCTSlider onSlidingComplete event on iOS from a bubbling event to a direct (non-bubbling) event to match the non-bubbling type on android.
Note that in this case these seems like a bug. I will still explain the motivation and reasoning as this will come up in future diffs.
## Changelog
[Slider][BREAKING] Switch Slider onSlidingComplete event to a non-bubbling event on iOS to match Android
## Motivation:
The motivation here is that when we codgen the view configs, we'll need to unify all of the events and props across platforms for components that have the same name on iOS and Android.
In this case, the view configs (below) conflict for onSlidingComplete. On iOS this is under bubblingEventTypes, on Android this is under directEventTypes. We have code [here](https://fburl.com/3s1dahm2) in the react native renderer which ensures an event is not listed as both.
```
// iOS
const SliderViewConfig = {
bubblingEventTypes: {
onSlidingComplete: {
phasedRegistrationNames: {
captured: 'onChangeCapture',
bubbled: 'onChange'
}
}
},
directEventTypes: {
// None
},
validAttributes: {
// ...
}
};
```
```
// Android
const SliderViewConfig = {
bubblingEventTypes: {
// None
},
directEventTypes: {
onSlidingComplete: {
registrationName: 'onEventDirect'
}
},
validAttributes: {
// ...
}
};
```
## Solutions
There are three solutions to this issue:
1. Don't generate view configs
2. Rename the component on one platform
3. Make a breaking change in the event behavior on one platform to make it consistent across both platforms
Here we've chosen option #3
Reviewed By: TheSavior
Differential Revision: D15322304
fbshipit-source-id: ff1ab86efe9e2bc50fd3f7619e6760ab5c1c5090
Summary:
Some ObjC NativeModules conform to `RCTInvalidating` protocol and implements `invalidate` method. This is typically used to clean things up during bridge teardown or reload. In TurboModule system the invalidate method can be replaced by pure destructors, but for backward compatibility, we call existing invalidate method on each module during teardown.
This also cleans up all existing LongLivedObject's.
Reviewed By: mdvacca, RSNara
Differential Revision: D15365655
fbshipit-source-id: 802844b39b5b7adb54970ea541f4d744bbf9e480
Summary:
With the introduction of TurboModules, it would be beneficial to measure the setup time of these modules, as we currently have it in place for NativeModules.
The instantiation of the TMs occurs in the `RCTTurboModuleManager`. In order to successfully measure the time it took to setup the module, we need to ensure that we don't take into account cached modules. As such, we need to:
1. Check if module is in `_turboModuleCache`
a. Start mark for `RCTPLTurboModuleSetup` tag if not found
2. Get the TM via `[self provideTurboModule:]`
3. Check if module is in `_turboModuleCache`
a. Stop mark for `RCTPLTurboModuleSetup` if we did not find module in cache prior to **step 2** and if it's now present in the cache.
b. Notify about setup time if the above is true.
4. Return TM
## Changelog
[iOS] [Added] - Gain insights on the the turbo module setup times by observing `RCTDidSetupModuleNotification`. The userInfo dictionary will contain the module name and setup time in milliseconds. These values can be extracted via `RCTDidSetupModuleNotificationModuleNameKey` and `RCTDidSetupModuleNotificationSetupTimeKey`.
Pull Request resolved: https://github.com/facebook/react-native/pull/24732
Differential Revision: D15362088
Pulled By: RSNara
fbshipit-source-id: e6a8044e4aba5a12ae63e9c7dbf707a17ec00180
Summary: Future improvements in RCTScrollViewComponentView require ability to have multiple listeners of UIScrollViewDelegate (e.g. PullToRefresh component needs it), therefore we need a splitter to support it.
Reviewed By: JoshuaGross
Differential Revision: D15345301
fbshipit-source-id: 62bb50c4fdd4fa64ece5d7cc6ddc76367c84c4b3
Summary:
This is the final piece of change that makes measuring (`LayoutableShadowNode::getRelativeLayoutMetrics()`) take ScrollView content offset into account (on iOS).
It works pretty simply: at the end of scrolling (or zooming) action ScrollView updates the state which later can be used for computing `transform` which measuring uses to adjust values in LaoutMetrics.
Reviewed By: mdvacca
Differential Revision: D15323688
fbshipit-source-id: fdf86c6cd9bdfd56caddd4b39bdd1185760b9f94
Summary: Seems we need this now to enable future improvements in ScrollView such as correct measure, pull-to-refresh and so on.
Reviewed By: mdvacca
Differential Revision: D15323687
fbshipit-source-id: fae37431ccbbf2faec9c84752396153689b873ef
Summary: We don't need it anymore because we don't use folly::Future.
Reviewed By: JoshuaGross
Differential Revision: D15344961
fbshipit-source-id: a368e600d69e6879f6a98b4a997de85c71186e92
Valentin Shergin
Mehdi Mulani
Tim Yung
zhongwuzw
Jim Berlage
Eric Lewis
Sharon Gong
Alex Cohen
spdr admin
Leo Natan
Marc Mulcahy
Samuel Susla
Lucas Bento
Kyle Fang
Rick Hanlon
Kevin Gozali
RCiesielczuk