Summary:
Support RCTModernEventEmitter+RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26054855
fbshipit-source-id: 9592d854962d53f64d836b8361256cac5c4325df
Summary:
Support RCTModernEventEmitter +RCTEventEmitter in an Event class(es). This improves perf in Fabric. Migrate any constructor callsites to the new constructor and deprecate the previous one.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26043861
fbshipit-source-id: 37757508c835cbd1181f0e0e774abc62fdbfee2b
Summary:
The `Event` interface has been improved such that:
1. `getEventData` is now a default method on Event that returns null
2. `dispatch` has a default implementation that relies on getEventName() and getEventData()
3. `dispatchModern` can detect if surfaceId and event data are present; if not, it falls back to dispatch
This will dramatically ease future migrations: at some point in the (distant) future, we can simply delete RCTEventEmitter and
all use-cases will be supported by the current `Event` class without needing to introduce a 3rd transitional interface; and 99%
of all Event classes can be simplified, delet their `dispatch` implementation and need no further work.
At their core, all Events are simply: (1) a name, (2) data, (3) a target (surfaceId and tag). The interface now reflects that but still
allows for flexibility of the data and names being generated on-demand if necessary; but for the vast majority of Event classes, code
will be dramatically simplified.
I also migrate a single Event class, ContentSizeChangeEvent, to use this new method of dispatch.
Changelog: [Android][Changed] Added convenience methods to simplify native Event classes and ease migrations
Reviewed By: mdvacca
Differential Revision: D26043325
fbshipit-source-id: bc308105f7f6e654d45fd156dbf4a2bcbc45819c
Summary:
See title. dispatchV2 was just introduced yesterday, so nothing relies on it yet / it's safe to rename (there are no released versions of RN with `dispatchV2`).
And... dispatchModern is a better name.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26039008
fbshipit-source-id: 009ae721b8caef23a389c33b51f6f8952a6a73da
Summary:
Support RCTEventEmitterV2 in ReactTextInput*Event classes for optimal Fabric perf. Backwards-compatible with non-Fabric renderer.
Changelog: [Changed][Android] Old native ReactTextInput*Event creation APIs have changed, and will be removed in the (distant) future. The old methods will work for now with minor perf implications for Fabric.
Reviewed By: mdvacca
Differential Revision: D26031261
fbshipit-source-id: 7c972ecfd1e395104c4639995bb5ecc5f7c6baae
Summary:
Support RCTEventEmitterV2 in ImageLoadEvent for optimal Fabric perf. Backwards-compatible with non-Fabric renderer.
Changelog: [Changed][Android] Old native ImageLoadEvent creation APIs have changed, and will be removed in the (distant) future. The old methods will work for now with minor perf implications for Fabric.
Reviewed By: mdvacca
Differential Revision: D26029773
fbshipit-source-id: c8e00e06a2f9d6682367f9099bdf7f5fc12890e0
Summary:
Motivation: perf, simplicity, adhering to new SurfaceMountingManager APIs available to us. Backwards-compatible with events sent through old system or Fabric, to Fabric or non-Fabric Views.
Changelog: [Changed][Android] Old Native method to create ScrollEvent has been deprecated and will be removed at some point in the (distant) future
Reviewed By: mdvacca
Differential Revision: D26027105
fbshipit-source-id: b9dba5b56c2bfed3b8fc4488c54b271b85ab5fa0
Summary:
There's a field called `surfaceID` in a couple of classes that isn't the same as the integer `surfaceId` in Fabric.
For consistency, I've deprecated a couple of them, or renamed when appropriate.
In addition, now we're passing the actual integer surfaceId into the ThemedReactContext. This means that every single View created in Fabric gets annotated with the surfaceId it's in. Currently this isn't used, but the idea is that now each View has a mapping back to its surface, which could be used to simplify / optimize operations with SurfaceMountingManager. In particular, we might be able to use this in the future to optimize animations and/or event emitters.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D26021571
fbshipit-source-id: b7db7de123db07fa928a6f815be86bdbb030e62c
Summary:
This diff changes the type of the SwipeRefreshLayoutManager.size prop from Int to String in Fabric.
The current implementation of this prop allows JS developers to use "int" type when fabric is enables and "int or string" types when using Fabric is disabled.
Since long term we want to only support "string" type for this prop, I'm changing the type of the prop to be String.
After my diff Fabric will start supporting only "string" types, non fabric screens will keep supporting "int or string" values.
**Will this break production?**
No, because there are no usages of RefreshControl.Size prop in fbsource
**What about if someone start using this prop next week?**
IMO It's very unlikely because of the nature of this prop, I will be monitoring next week and if there's an usage it will be detected by flow when trying to land D25933457.
Changelog: [Android][Changed] - RefreshControl.size prop changed its type to string, the valid values are: 'default' and 'large'
Reviewed By: JoshuaGross
Differential Revision: D25933458
fbshipit-source-id: 55067d7405b063f1e8d9bb7a5fd7731f5f168960
Summary:
This diff migrates all the lookups of EventDispatcher to not depend on UIManagerModule anymore.
This refactor is necessary because:
- Users running in Fabric / Venice should not load on the UIManagerModule class
- D25858934 will introduce a change that will break all of these callsites
In the migration I'm relying on the method UIManagerHelper.getEventDispatcherFromReactTag() that returns the correct EventDispatcher for a reactTag.
I'm planning to land this change early in the week (to catch potential errors in alpha / beta versions)
As a followup we need to deprecate and prevent developers to continue using getNativeModule(UIManagerModule.class) moving forward. That will be part of another diff
changelog: [internal] internal
Reviewed By: JoshuaGross
Differential Revision: D25858933
fbshipit-source-id: e26c99759307517b5bef483274fe0e0d71bb4c6c
Summary:
Right now we assume in the Image component that any prop changes requires a redraw of the image, even if the props set are identical.
Noop prop updates can be caused in Fabric by LayoutAnimations. This may go away in the future, but only when we have a new animations system.
I don't think most other components need to be concerned with this, and many other components already guard against unnecessary redraws. Since the image "flashes"
when it is loaded, unlike most other native components, this issue is more noticeable for images.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25727482
fbshipit-source-id: 75ffa456bddc1208900733140ce4ff19f7e2c11e
Summary:
For core components, we can start using the codegen output during build time instead of the checked in files in: https://github.com/facebook/react-native/tree/master/ReactAndroid/src/main/java/com/facebook/react/viewmanagers
Note: Some files seemed to be handwritten, so this only removed those that use codegen.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D25453157
fbshipit-source-id: f7eabddfd3fd668bef0c4aef3fddcb38c8b046a0
Summary:
In preparation for flipping the default, marking autoglob as False in places where it isn't explicitly specified.
Changelog: [Internal]
Reviewed By: strulovich
Differential Revision: D25497305
fbshipit-source-id: 142e5caca2d67efcb3c25067a36934f7f6dd4b21
Summary:
The flag `clipChildRectsIfOverflowIsHidden` has been set to false for a little over a year. Delete it and associated callsites.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25451696
fbshipit-source-id: a6067b2e25124f6bdef336c2ddead719dd44cca9
Summary:
This has been true for 100% of users for months; clean up the old code.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25451470
fbshipit-source-id: feae59ce746869b9d84d6aaa69be10e91181f03a
Summary:
This code was useful for debugging T78035906 but is no longer needed.
I'm a bit conflicted about removing it, since such issues could crop up again in the future - but they're very rare, and hopefully now we know how to avoid them.
tl;dr: Make sure that mount instructions are not executed *during* a draw or measure.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25450846
fbshipit-source-id: e261f13db252d22773e373a5de744ecc8c380a43
Summary:
This code and this class isn't needed in Fabric, so don't create it during construction. Lazy-load it when it's needed outside of Fabric.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D25450724
fbshipit-source-id: d14d8f03c194716f2aba0e499f3282ad2d1c1d29
Summary:
Updates `ReactScrollViewManager` and the `ViewConfig` for `ScrollView` so that they are equivalent.
- `inverted` was missing.
- `contentOffset` was missing differ on Android. (However, there does not seem to be any perceivable behavior difference besides the native `ViewConfig` being different.)
Changelog:
[Internal]
Reviewed By: JoshuaGross
Differential Revision: D25084470
fbshipit-source-id: 8bea3b7a692c1038819a4147b174583a4faa71e9
Summary:
We don't support pre-API 21/Android <5/pre-Lollipop anymore; delete related code or checks.
No need for this to make it to the changelog, this announcement was already made.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D24965249
fbshipit-source-id: e537e62e8eb18878c29afe17b8c7861d095a37b7
Summary:
This pr fixes: https://github.com/facebook/react-native/issues/30123 .
When secureTextEntry is true, setInputType will set the inputType of textInput to password type.
Password type default font-family will be monospace font, so we need to setTypeface after the setInputType.
## Changelog
[Android] [Fixed] - Font family is not apply when secureTextEntry is true.
Pull Request resolved: https://github.com/facebook/react-native/pull/30164
Test Plan:
Before this pr:
After this pr:
Please initiated a new project and replaced the App.js with the following code:
```
iimport React from 'react';
import {SafeAreaView, TextInput} from 'react-native';
const App = () => {
return (
<SafeAreaView>
<TextInput
id={'email'}
placeholder={'Email'}
secureTextEntry={false}
style={{fontFamily: 'Helvetica', fontSize: 14, fontWeight: '400'}}
/>
<TextInput
id={'password'}
placeholder={'Password'}
secureTextEntry={true}
style={{fontFamily: 'Helvetica', fontSize: 14, fontWeight: '400'}}
/>
</SafeAreaView>
);
};
export default App;
```
Thanks you so much for your code review!
Reviewed By: cpojer
Differential Revision: D24686222
Pulled By: hramos
fbshipit-source-id: 863ebe1dba36cac7d91b2735fe6e914ac839ed44
Summary: We support setting the text color in the ReactPicker component, but we don't apply the text color to the little arrow icon that appears next to it. This diff applies the color tint from the picker's primary text color (set with a style prop on the main picker component, *not* the 'color' prop on the Picker.Item) to the background of the picker, which tints the arrow icon.
Reviewed By: makovkastar
Differential Revision: D24480642
fbshipit-source-id: 7ce84d616ae677da8975be9444428392020c57dc
Summary:
Fabric should be inserting Views into the hierarchy in the correct order based on z-index already, so there should be no reason to enable this mechanism.
At best it's a perf pessimisation and at worst it could be causing consistency issues or crashing (TBD). Most likely this is a noop.
Changelog: [Internal]
Reviewed By: ejanzer
Differential Revision: D24512203
fbshipit-source-id: b9336240ef8506742bcbd8d08fc8b830f82cdfe2
Summary:
I want to see if the child count changes before/after this method execute; that would help pinpoint the cause of these crashes.
Changelog: [Internal]
Reviewed By: yungsters
Differential Revision: D24510064
fbshipit-source-id: 11b4baf15bc5e0beb23d65546605b378d84d1b20
Summary:
The value of the `ScrollView.snapToOffsets` property can be an empty array (most likely an issue in the product code), which will crash the app. This diff adds a check to prevent crashing in this scenario and falling back to the default snap behaviour.
Changelog:
[Android][Fixed] - Do not crash when ScrollView snapToOffsets is empty
Reviewed By: sammy-SC
Differential Revision: D24502365
fbshipit-source-id: c63b8e3b8f2fb323ebd6c962ee628015934d8e11
Summary:
There's a crash for a small number of users that looks like it is happening when cutting the text via a context menu, or deleting content near the end.
This is only happening because we cache the Spannable and it detects changes due to the cache mechanism itself. I'm making a minor change that will hopefully result
in Spannables being copied instead of the same Spannable instances being used for display on the View and measurement; and also swallowing this error, since it should
not be considered as a fatal, unrecoverable error for now. Hopefully we can delete entirely in the future.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24430622
fbshipit-source-id: 495458b3d85733e46a7e62b5c954b7cb6b00470b
Summary:
We've deprecated API 20 and below. This is just a cleanup to remove code that assumes API level <21.
Changelog: [Android][Deprecated] Deprecate support of Android API levels 19 and 20.
Reviewed By: fkgozali
Differential Revision: D24380233
fbshipit-source-id: d8f98d7cb19446a60ba36137f1f9290e35f27c02
Summary:
dispatchDraw, dispatchGetDisplayList, updateDisplayListIfDirty, recreateChildDisplayList, etc, can all crash internally for a variety of reasons and it can be very tricky to track down the root cause. This isn't a fix, this just adds extra logging to hopefully make debugging easier.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D24166149
fbshipit-source-id: 1bbaf34a92a9bcac5a594a25522c66e6e0cc80ca
Summary:
When I landed D24042677 (https://github.com/facebook/react-native/commit/030d2c1931fb9ff97f682343914503a1c359e1c4), I had the right idea in spirit but forgot to negate the if statement. Oops.
This means that in non-Fabric, the cached spannable will be updated (potentially causing a crash) and the cached spannable will *never* be updated, meaning that most TextInputs will be measured as 0 in Fabric.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24119952
fbshipit-source-id: dc86137956535e1f2b147bb432d050b3561e2658
Summary:
This is a resubmit of D21499015. It resolves https://github.com/facebook/react-native/pull/28852 and https://github.com/facebook/react-native/issues/27787.
From Harry Yu's original PR (too old to merge now):
Text in TextInputs can't be selected by long press. This happens only when they're inside of FlatLists that are rendered with removeClippedSubview prop set to true on Android.
Fixes https://github.com/facebook/react-native/issues/27787
Issue https://github.com/facebook/react-native/issues/6085 2 years ago had fixed this issue with a quick fix, but the code has since disappeared in another change. It has a longer explanation for why it's fixed, but essentially - the text is assumed to be not selectable since the TextInput is initialized without being attached to the window. We need to explicitly set the text to be selectable on attachment. This change redoes that change with a 1-line fix.
Changelog: [Android] [Fixed] - Fixed TextInput not being selectable in removeClippedSubviews FlatLists
Pull Request resolved: https://github.com/facebook/react-native/pull/28852
Test Plan:
This can be tested with a new toggle in RNTesterApp.
Go to the FlatList in RNTesterApp
Toggle on "removeClippedSubviews"
Try selecting some text in the list header. It should fail without this comment but succeed with it
Reviewed By: sammy-SC
Differential Revision: D24043533
Pulled By: JoshuaGross
fbshipit-source-id: c8e60f8131ccc5f6af31ed976c4184d0a16eb3af
Summary:
This is a check to execute code only in Fabric, and... it's just wrong. This object is *always* present, for Fabric and non-Fabric. We instead need to check if there's actually a state object, as other parts of the code check for.
Changelog: [Internal]
Reviewed By: sammy-SC
Differential Revision: D24042677
fbshipit-source-id: 5cf6ebc8f07987d917fdf11042d1715876fa8229
Summary:
in Android 11, there's an issue where Text content is being clipped. The root cause appears to be a breaking change in how Android 11 is measuring text. rounding up the layoutWidth calculation mitigates the issue.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D23944772
fbshipit-source-id: 1639259da1c2c507c6bfc80fed377577316febac
Summary:
Changelog: [internal]
When paper measures text, it sets `useLineSpacingFromFallbacks` flag on the Layout object. In Fabric this was missing and can cause incorrect layout.
Reviewed By: JoshuaGross
Differential Revision: D23845441
fbshipit-source-id: 538f440cdbbf8df2cba0458837b80db103888113
Summary:
Changelog: [Internal]
Construction of Layout will be needed in `TextLayoutManager.measureLines`, pulling it out into separate function prevents code duplication.
Reviewed By: shergin
Differential Revision: D23782905
fbshipit-source-id: 8ab817559ca154716a190ca1012e809c5fa2fd6e
Summary:
Right now nested Text components are not accessible on Android. This is because we only create a native ReactTextView for the parent component; the styling and touch handling for the child component are handled using spans. In order for TalkBack to announce the link, we need to linkify the text using a ClickableSpan.
This diff adds ReactClickableSpan, which TextLayoutManager uses to linkify a span of text when its corresponding React component has `accessibilityRole="link"`. For example:
<Text>
A paragraph with some
<Text accessible={true} accessibilityRole="link" onPress={onPress} onClick={onClick}>links</Text>
surrounded by other text.
</Text>
With this diff, the child Text component will be announced by TalkBack ('links available') and exposed as an option in the context menu. Clicking on the link in the context menu fires the Text component's onClick, which we're explicitly forwarding to onPress in Text.js (for now - ideally this would probably use a separate event, but that would involve wiring it up in the renderer as well).
ReactClickableSpan also applies text color from React if it exists; this is to override the default Android link styling (teal + underline).
Changelog: [Android][Fixed] Make nested Text components accessible as links
Reviewed By: yungsters, mdvacca
Differential Revision: D23553222
fbshipit-source-id: a962b2833d73ec81047e86cfb41846513c486d87
Summary:
This fixes TextInput measurement caching. Previously we were not setting the correct Spannable in the cache; we needed to additional Spans to it to indicate font size and other attributes.
This brings Fabric closer to how non-Fabric was measuring Spannables for TextInputs (see ReactTextInputShadowNode.java).
This should fix a few crashes and will be most noticeable with dynamically-sized multiline textinputs where the number of lines changes over time.
This also allows us to transmit less data from C++ to Java in the majority of cases.
Changelog: [Internal]
Differential Revision: D23670779
fbshipit-source-id: cf9b8c848b9e0c2619e01766b72b074248466825
Summary:
Right now you can scroll a horizontal ScrollView with TalkBack even if you've disabled scrolling in JS, because our HorizontalScrollView component doesn't prevent the accessibility scroll event (this doesn't seem to happen with vertical ScrollViews for some reason...) This diff adds an accessibility delegate to both of the Android ScrollView components to make sure they're not scrollable if scrolling has been disabled in JS.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D23582689
fbshipit-source-id: b670bdb462ab9c963c7125597d60ca97c7d88a9c
Summary:
When we render a text input that already has a text value (<TextInput value="123" />), its selection (cursor) is automatically set to the end of the text. However, when you swipe to focus the text input with TalkBack, Android decides it needs to clear the selection, which moves the cursor back to the beginning of the text input. This is probably not what you want if you're editing some text that's already there. Ideally we would just keep the selection at the end, but I don't know how to prevent this from happening - it seems to be part of how TextView handles the accessibility focus event? So instead I'm just explicitly setting the selection to the end of the text in our handler for accessibility click.
Changelog: [Android][Fixed] Move selection to the end of the text input on accessibility click
Reviewed By: mdvacca
Differential Revision: D23441077
fbshipit-source-id: 16964f5b106637e55a98c6b0ef0f0041e8e6215d
Summary:
After monitoring scuba for a few days, previous fixes(D23301714 D23331828 (https://github.com/facebook/react-native/commit/07a597ad185c8c31ac38bdd4d022b0b880d02859)) don't work as expected.
I managed to test this issue on a Xiaomi device, the crash didn't happen but the there was a popup "Frequetly used email" on top of email edit text:
{F317216473}
Getting rid of the popup probably be the right fix.
For more context see https://github.com/facebook/react-native/issues/27204
Changelog: [Android] - Set caretHidden to true to fix the Xiaomi crash
Reviewed By: mdvacca
Differential Revision: D23451929
fbshipit-source-id: 521931422f3a46a056a9faa4b10fe93cf4732db0
Summary:
This diff optimizes the caching of Spannable objects managed by the TextLayoutManager class.
Previously, these objects were cached using unsing a String representation of the RedableMap (creating this string adds a non trivial cost), this diff improves the caching performance relying on the equals / hashcode methods of the ReadableNativeMap class
I created a MC just to have a killswitch
Motivation: I was analysing another bug and I found this non performant code
changelog: [internal] internal
Reviewed By: shergin
Differential Revision: D23429365
fbshipit-source-id: 59e5ad0b1b95da992ac393aecfe029da68a8df97
