Commit Graph

104 Commits

Author SHA1 Message Date
Joshua Gross a65cd683ae Align View creation flow between Fabric and non-Fabric
Summary:
Ship responsibility for most View creation logic to ViewManager, where it already largely lies, and simplify code in Fabric and non-Fabric mounting layers.

Notably, some of this work was *already* being duplicated so we can expect an extremely tiny perf gain here.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D26742711

fbshipit-source-id: 4213766d4cd366bc69cd47d4654f7b269bb9e7f4
2021-03-04 22:22:34 -08:00
Gaurav Gupta ea3495399b Android - pass initial props to ViewManager createView in non-Fabric (#31053)
Summary:
https://github.com/facebook/react-native/issues/31051
When trying to create custom viewmanagers,  we don't have props, I have a use case where I want to create views on the basis of provided react prop from react native.
## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Changed] - pass initial props to ViewManager createViewInstance method in non-Fabric

Pull Request resolved: https://github.com/facebook/react-native/pull/31053

Test Plan:
Tested Manually.

(Facebook - see D26719538, which should land first)

Reviewed By: mdvacca

Differential Revision: D26719510

Pulled By: JoshuaGross

fbshipit-source-id: ced78aa919e6b433e22ddb7c9eccc3e3e91950e9
2021-03-04 22:22:34 -08:00
David Vacca 2e63147109 Refactor creation of views in Fabric Android
Summary:
This diff refactors the createViewInstance method in order to ensure that viewID is set before props are updated in the view.
This is necessary because there are components that deliver events at the same time their props are set. This means that some components might not have their viewId set correctly when events are delivered.
Since viewId is used to determine if a view belongs to Fabric or Paper, there are cases when the events are not delivered to the right renderer

changelog: [internal]

Reviewed By: JoshuaGross

Differential Revision: D25667987

fbshipit-source-id: 4acfa8f80d66e9e59514354481957d7d3b571248
2021-01-05 23:28:28 -08:00
Joshua Gross ca4bac5534 EZ: Fix debug mode NPE in non-Fabric
Summary:
Fixes a NPE in debug mode. This will only impact developers who have explicitly turned this debug flag on, so it's a very low-pri fix.

Changelog: [Internal]

Reviewed By: makovkastar

Differential Revision: D24410825

fbshipit-source-id: 08c8a0c6d0e0fb7c132725ad6af9460b91a7edf3
2020-10-20 05:20:26 -07:00
Joshua Gross 3aa4de7912 Add debugging mode to non-Fabric NativeViewHierarchyManager
Summary:
Just helpful for debugging sessions.

Changelog: [Internal]

Reviewed By: sammy-SC

Differential Revision: D24207622

fbshipit-source-id: 904cbaa4512c03581d6a5c3efd69ebfca7972d42
2020-10-09 12:16:50 -07:00
Joshua Gross 86c38739a7 Avoid using ViewManager childCount to detect errors, since it's not reliable
Summary:
Similar to D21756178, we cannot rely on childCount since it can return zero when there are actually valid children. This both causes more exceptions than necessary when the operation would work, and pollutes error messages since the information is not strictly reliable.

Instead, we just try to get a View and thrown an exception when it's null, or in loops, loop until we hit a null child. `getChildAt` doesn't throw exceptions, it just returns null when we're out-of-bounds.

This can impact custom ViewGroups like BottomSheets, and other ViewGroups that might do interesting/weird things with children, including ReactClippingViewManager.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22035569

fbshipit-source-id: 43e98d81178faaf720face98fc84e78743f292c3
2020-06-13 18:31:41 -07:00
Lulu Wu 63099c40e6 Migrate Android view managers to type-safe commands generated by JS codegen
Summary:
## Changelog:

[General] [Changed] - Migrate Android view managers to type-safe commands generated by JS codegen.

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D21406461

fbshipit-source-id: 93584b240314254675a36a58c4d0c0880d6889fb
2020-05-12 04:37:44 -07:00
Jakub Kinst b020e7c440 Fix calculating View position within a Window in split-screen mode on Android (#28449)
Summary:
On Android, when using split-screen mode, the Window-relative position of a native View is not calculated properly when the app is running on the right (or bottom) half of the screen. The coordinates are currently calculated only based on View.getLocationOnScreen() with subtracting status bar height. Scenarios, where the window does not fill the entire screen (such as split-screen mode) are not supported.

We need to use a more general solution to subtract the actual position of the window from position of the view within the screen. This PR fixes the issue by subtracting coordinates of the Window retrieved from View.getWindowVisibleDisplayFrame(), which covers all scenarios when Window can be in a different position than the screen (incl. status bar offset).

## Changelog

[Android] [Fixed] - Calculating view position within the window in split-screen mode
Pull Request resolved: https://github.com/facebook/react-native/pull/28449

Test Plan:
1. Run an app in split-screen mode on the right half of the screen
2. Call UIManagerModule.measureInWindow() from JS to fetch a position of a View within a Window
3. Observe the wrong coordinates returned
4. Try the same with the fix

Reviewed By: mdvacca

Differential Revision: D21246297

Pulled By: shergin

fbshipit-source-id: 1f54b1a5d6610be17bf05521200304db2ba263ab
2020-04-26 22:14:33 -07:00
Joshua Gross eab7fc008f Remove unused feature flag: logDroppedViews
Summary:
Remove unused internal feature flag, logDroppedViews.

Changelog: [Internal]

Reviewed By: lunaleaps

Differential Revision: D20797353

fbshipit-source-id: 1bfea7fcce9e80cdb92cda59a89c7dd817d4a581
2020-04-01 17:07:07 -07:00
Luna Wei 44f213b40e Modify pending deletion tags to be cross manageChildren
Summary:
Changelog: [Internal]

Removing historic layout animations index adjustment (D20323928) broke the Dating Secret Crush screen.

Since flushing animations (D20319824) had to be reverted due to issues with Saved + Privacy Shortcuts (https://fburl.com/tasks/eijtmifu) we need to track pending deletions across `manageChildren` operations.

Reviewed By: JoshuaGross

Differential Revision: D20601079

fbshipit-source-id: c6f116683750e97abe7f988cf361d2a6449e90e6
2020-03-23 17:36:13 -07:00
Joshua Gross 0fe548aa2a Only retry ViewCommand mount items if exception is marked as "Retryable"
Summary:
Instead of just blindly retrying all ViewCommands if they fail - which could be dangerous, since it's arbitrary imperative commands we'd be executing twice, potentially with bad app state - we only retry if the ViewCommand throws a "RetryableMountingLayerException".

Changelog: [Internal] Optimization to ViewCommands

Reviewed By: mdvacca

Differential Revision: D20529985

fbshipit-source-id: 0217b43f4bf92442bcc7ca48c8ae2b9a9e543dc9
2020-03-19 23:02:04 -07:00
Ramanpreet Nara 950826f6b5 Back out "Track animations and flush them"
Summary:
Original commit changeset: b594d0e6e9b6

D20319824 introduced a problem in LayoutAnimations, which makes surfaced as the problem in T63911344. This diff reverts D20319824.

Changelog: [Internal]

Reviewed By: lunaleaps

Differential Revision: D20541918

fbshipit-source-id: ff72b839f57d39051122920a38b2632cbb5ec362
2020-03-19 20:49:36 -07:00
Joshua Gross bb7f4dc818 Early ViewCommand dispatch: retry exactly once, log soft errors in one place
Summary:
As a followup to T63997094, I think it is better to attempt to execute early once, and then to execute the ViewCommand after the current batch of mount instructions if that fails. If both fail, then log a soft exception.

This is to support the use-case here, when ScrollView.scrollToEnd is called during the render pass, before any Views are mounted on the screen: https://our.intern.facebook.com/intern/diffusion/FBS/browse/master/xplat/js/RKJSModules/Apps/Profile/ProfileEdit/ui/featured_highlights_migration/ProfileEditFeaturedHighlightsMigrationProfilePreview.js?commit=75f66a9077abb81b7797079b567df759dd023a79&lines=221-229

Changelog: [Internal] Fix for dispatching ViewCommands early

Reviewed By: mdvacca

Differential Revision: D20478681

fbshipit-source-id: 052b3ecf4913a0096f590637faf0f68a072e2427
2020-03-16 19:41:47 -07:00
Joshua Gross 5296a740a7 Dispatching ViewCommands to a non-existent tag should log SoftException, but not crash
Summary:
Patch for T63997094.

Will still crash in debug mode, and log soft errors, so as not to entirely hide errors.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D20473855

fbshipit-source-id: 8b052b1ae3c886f83d6a7922feb158172cdcd33d
2020-03-16 13:28:08 -07:00
Luna Wei c938c0afbf Adjust Layout Animation with pending deletion set
Summary:
Changelog: [Internal]
This solves the problem of viewToAdd indices being invalid if the deletes are asynchronous within the scope of one `manageChildren` call.

Since deletions are performed first, we keep a set of tags being deleted.
During view insertion, we iterate through and filter those tags out of our count.

Reviewed By: JoshuaGross

Differential Revision: D20324643

fbshipit-source-id: 150230428fcd65b8c43cc1f2331e9ce02d31fff9
2020-03-09 18:33:53 -07:00
Luna Wei dedf9372ca Track animations and flush them
Summary:
Changelog: [Internal]

Track delete animations and when `manageChildren` is called on a certain view tag, finish all pending deletion animations before manipulating children

Reviewed By: JoshuaGross

Differential Revision: D20319824

fbshipit-source-id: b594d0e6e9b6fecc5eca2938f284be631494e55c
2020-03-09 15:56:38 -07:00
Luna Wei d3b93f7578 Remove all layout index adjustments
Summary:
Changelog: [Internal]

# Context Timeline

* ~March 2019 landed D14529038 (I'll be referring to this as "index adjustment fix")
which attempted to fix a reproducible issue with layout animations: P127130177, see Spencer's diff for more context: D14245985

* May 2019 I realized that "index adjustment fix" has a bug in it and attempted to fix with D15485132, but was eventually reverted because of other crashes

* Just recently have been getting tasks related to crashes that are attempting to either remove or add a view that is out of bounds which is caused by invalid index because of the "index adjustment fix".

# What is this diff doing?
I'm removing the "index adjustment fix" because I found that the original layout animation repro, P127130177, no longer repros on  latest master with the "index adjustment fix" reverted.

Additionally, I've found a consistent crash in (RN bookmark > Sample Integration App > Relay Sample Friends) of a bad view deletion because of the "index adjustment fix"

Removing the index adjustment fix may have effects elsewhere but it seems better to remove this and go back to what layout animations was doing a year ago than to continue on in this half-baked state.

Reviewed By: JoshuaGross

Differential Revision: D20323928

fbshipit-source-id: ba4a222915add00e98a9936ba2a8efc4006fb8e3
2020-03-09 15:56:37 -07:00
Joshua Gross f77abc583c Add UI asserts and logs to a few more places
Summary:
Add UI asserts to ensure thread safety of some operations.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D18339981

fbshipit-source-id: 9162b6351f40bdd543d3e255691e9f54d1934589
2019-11-05 18:20:01 -08:00
Andres Suarez 3b31e69e28 Tidy up license headers [2/n]
Summary: Changelog: [General] [Fixed] - License header cleanup

Reviewed By: yungsters

Differential Revision: D17952694

fbshipit-source-id: 17c87de7ebb271fa2ac8d00af72a4d1addef8bd0
2019-10-16 10:06:34 -07:00
Florian Cargoët 28d50189f3 Fixed android bounding box (#25836)
Summary:
This PR fixes https://github.com/facebook/react-native/issues/19637.

Summary of the issue: on Android, transformed touchables have press issues because the touchable's measurements are incorrect in the release phase. `UIManager.measure()` returns an incorrect size and position as explained [here](https://github.com/facebook/react-native/issues/19637#issuecomment-396065914)

This is easily seen with the inspector :

**Screenshot of a { scale: 2 } transform**
The real view (scaled) is in pink, the incorrect touchable area is in blue.
<img src="https://user-images.githubusercontent.com/110431/41190133-8d07ad02-6bd9-11e8-873d-93776a007309.png" width="200"/>

**Screenshot of a { rotateZ: "-45deg" } transform**
The real view (rotated) is in pink, the incorrect touchable area is in blue.
<img src="https://user-images.githubusercontent.com/110431/41190136-a1a079a6-6bd9-11e8-906d-729015bcab6b.png" width="200"/>

## Changelog

[Android] [Fixed] - Fix UIManager.measure()
Pull Request resolved: https://github.com/facebook/react-native/pull/25836

Test Plan:
Android now produces the same results as iOS as seen on these screenshots

| Android without fix | Android with fix | iOS |
| --- | --- | --- |
| ![Screenshot_1564133103](https://user-images.githubusercontent.com/110431/61941632-28729b00-af98-11e9-9706-b13968b79df5.png) | ![Screenshot_1564130198](https://user-images.githubusercontent.com/110431/61941631-28729b00-af98-11e9-9ff3-5f2748077dbe.png) | ![Simulator Screen Shot - iPhone X - 2019-07-26 at 11 19 34](https://user-images.githubusercontent.com/110431/61941633-28729b00-af98-11e9-8dc4-22b61178242e.png) |

Reviewed By: cpojer

Differential Revision: D16598914

Pulled By: makovkastar

fbshipit-source-id: d56b008b717ea17731fb09001cbd395aa1b044fe
2019-08-05 04:42:50 -07:00
David Vacca aa5edca0e2 Migrate Nullable and NonNull annotations to AndroidX
Summary:
This diff migrates the usages Nullable and NonNull annotations to AndroidX instead of javax.

The purpose of this change is to bring consistency in the annotations used by the core of RN

Reviewed By: makovkastar

Differential Revision: D16054504

fbshipit-source-id: 21d888854da088d2a14615a90d4dc058e5286b91
2019-07-11 16:23:29 -07:00
David Vacca a679f592cd Remove warnings in some RN Android classes
Summary: I've been analyzing some issue in RN Android code and I noticed some warnings to clean up

Reviewed By: ejanzer

Differential Revision: D16060522

fbshipit-source-id: 327fa86c24c7dd67ac2376bbd2f0ca4339f106d1
2019-07-02 14:47:21 -07:00
David Vacca 6eec39313d Deprecate receiveCommand method for INT commands
Summary:
This diff deprecates:

```
  public void receiveCommand(int reactTag, int commandId, Nullable ReadableArray commandArgs) {

```
in favor of:

```
  public void receiveCommand(int reactTag, String commandId, Nullable ReadableArray commandArgs) {
```

Reviewed By: JoshuaGross, TheSavior

Differential Revision: D16019254

fbshipit-source-id: 61efefe5d5c43f9b24b729f17229725b87b60a1f
2019-07-02 14:47:20 -07:00
Oleksandr Melnykov cd05a85fe5 Fix Javadocs broken by google-java-format
Summary: After we ran google-java-format D16071725, some Javadocs which weren't properly written broke. This includes putting unordered and ordered lists not using <ul> and <ol>, putting code blocks and pseudo-graphics not using <pre>. I ran through all the changed classes and tried to fix the broken Javadocs.

Reviewed By: cpojer

Differential Revision: D16090087

fbshipit-source-id: f31971cbc0e367a04814ff90bbfb2192751d5e16
2019-07-02 09:39:21 -07:00
Oleksandr Melnykov 6c0f73b322 Format Java code in xplat/js/react-native-github
Summary:
This diff formats the Java class files inside xplat/js/react-native-github. Since google-java-format was enabled in D16071401 we want to codemode the existing code so that users don't have to deal with formatter lint noise at diff-time.

```arc f --paths-cmd 'hg files -I "**/*.java"'```

drop-conflicts

Reviewed By: cpojer

Differential Revision: D16071725

fbshipit-source-id: fc6e3852e45742c109f0c5ac4065d64201c74204
2019-07-02 04:16:46 -07:00
Eli White 3cae6fa950 RN Android: Support View Manager Commands that are strings
Summary:
Right now JS triggers a view manager command with the following code:

```
UIManager.dispatchViewManagerCommand(
  ReactNative.findNodeHandle(this),
  UIManager.getViewManagerConfig('RCTView').Commands.hotspotUpdate,
  [destX || 0, destY || 0],
);
```

As we want to get rid of calls to UIManager, we need to stop looking for the integer defined in native from JavaScript. We will be changing methods like this to be:

```
UIManager.dispatchViewManagerCommand(
  ReactNative.findNodeHandle(this),
  'hotspotUpdate',
  [destX || 0, destY || 0],
);
```

We need to support ints and Strings to be backwards compatible, but ints will be deprecated.

Reviewed By: shergin

Differential Revision: D15955444

fbshipit-source-id: d1c488975ae03404f8f851a7035b58a90ed34163
2019-06-24 18:47:16 -07:00
Luna Wei 545b084cc8 Back out "[RN] Layout Animation fix for normalized indices"
Summary:
Original commit changeset: 43e57dffa807

[Errors are growing and very ominious](https://our.intern.facebook.com/intern/logview/details/facebook_android_crashes/6acac038d7dd9d8ca95d1b9bccc4dfaa/?trace_key=39c72364f916998602bd55f091b04682)
I don't have a good idea what's going on.
This will re-introduce this bug: T44343673

Next steps:
* Would be great to try and get a repro to fix.
* Refactor this logic out to its own class and write actual tests.

Reviewed By: mdvacca

Differential Revision: D15822555

fbshipit-source-id: 0a2ec3d5c73420ca56aad93a4323a34cff1cc9c7
2019-06-14 07:43:06 -07:00
Charley 5b72ec31ae Modify comment of NativeViewHierarchyManager.java. (#25235)
Summary:
Modify the wrong word in the comment of NativeViewHierarchyManager.java.

Fix the explanation of the code to avoid misunderstanding.

## Changelog

[Android] [Fixed] - Fix the explanation of the code to avoid misunderstanding.
Pull Request resolved: https://github.com/facebook/react-native/pull/25235

Differential Revision: D15779249

Pulled By: cpojer

fbshipit-source-id: d4e7baa9ea9be5551feed8f643fe9774b3226bd8
2019-06-12 06:01:17 -07:00
Luna Wei bd3023abea Layout Animation fix for normalized indices
Summary:
[Android][Fix] - Fix how we normalize indices

Before, we were incorrectly normalizing indices given pending view deletion in the view hierarchy (namely, using LayoutAnimations)

What we had before (that was wrong):
* Maintained a pendingIndices sparse array for each tag
* For each pendingIndices sparse array we'd keep track of how many views we deleted at each abstract index
* Given an abstract index to delete a view at, we'd consult `pendingIndices` array to sum how many pending deletes we had for indices equal or smaller than and add to abstract index

^ Above algorithm is wrong and you can follow along with the following example to see how.

## The correct approach
Given these operations in this order:
1. {tagsToDelete: [123], indicesToDelete [2]}
2. {tagsToDelete: [124], indicesToDelete [1]}
3. {tagsToDelete: [125], indicesToDelete [2]}
4. {tagsToDelete: [126], indicesToDelete [1]}

The approach we want to be using to calculate normalized indices:
### Step 1: Delete tag 124 at index 2

|Views:|122|123|124|125|126|127|
|Actual Indices:|0|1|2|3|4|5|
|Abstract Indices:|0|1|2|3|4|5|
=> simple, we just mark the view at 2

### Step 2: Delete tag 123 at index 1
View tags and indices:
|Views|122|123|~~124~~|125|126|127|
|Actual indices|0|1|~~2~~|3|4|5|
|Abstract Indices|0|1||2|3|4|
=> again, simple, we can just use the normalized index 1 because no pending deletes affect this operation

### Step 3: Delete tag 126 at index 2
View tags and indices:
|Views|122|~~123~~|~~124~~|125|126|127|
|Actual Indices|0|~~1~~|~~2~~|3|4|5|
|Abstract Indices|0|||1|2|3|
=> Here we want to normalize this index to 4 because we need to account the 2 views that should be skipped

### Step 4: Delete tag 125 at index 1
View tags and indices:
|Views|122|~~123~~|~~124~~|125|~~126~~|127|
|Actual Indices|0|~~1~~|~~2~~|3|~~4~~|5|
|Abstract Indices|0|||1||2|
=> The normalized index should be 3.

This diff updates the function `normalizeIndex` to do the above algorithm by repurposing `pendingIndicesToDelete` to instead be a sparse int array that holds [normalizedIndex]=[tag] pairs
It's required that `pendingIndicesToDelete` is ordered by the normalizedIndex.

Reviewed By: mdvacca

Differential Revision: D15485132

fbshipit-source-id: 43e57dffa807e8ea50fa1650c5dec13a6fded624
2019-05-28 09:09:08 -07:00
Luna Wei 5979eafb16 Back out "[RN] Fix layout animation crash"
Summary: Original commit changeset: 41200e572ed7

Reviewed By: mdvacca

Differential Revision: D15485156

fbshipit-source-id: d0868a03b7186bb33998afc2c99dd85f31c8fef9
2019-05-28 09:09:07 -07:00
Blair Vanderhoof 9fb31d1538 Fix layout animation crash
Summary: As of D14529038, LayoutAnimations can sometimes throw an exception due to the view being null.  This can happen when elements are removed/added and is not fixable in product code. This is a temporary fix - the root cause for this issue will be fixed soon.

Reviewed By: lunaleaps

Differential Revision: D15428791

fbshipit-source-id: 41200e572ed7d5d470754792c5576a0ea23fe946
2019-05-22 13:25:00 -07:00
Joshua Gross b05761f1bb Pass State to preallocateView and createView methods whenever possible
Summary:
For some components, we will have state as soon as the ShadowNode is created that may be meaningful. In those cases, ViewManagers should be able to use State to create or preallocate views.

FB: This will be used in following diffs for Litho support.

Reviewed By: mdvacca

Differential Revision: D15343702

fbshipit-source-id: 8fd672251cb88dea662b5cae5a9efc96877d28a9
2019-05-21 15:05:18 -07:00
Luna Wei 0d7a0dc997 Add log to view that is being dropped
Summary:
We are getting errors where views are being dropped twice.
This is following from logic that viewManagers are only removed from `mTagsToViewManagers` from `dropView`.

This log will hopefully identify if we are getting improper operations because we shouldn't be re-using tags

Reviewed By: mdvacca

Differential Revision: D15152869

fbshipit-source-id: 914ee9c1772fa066adefde0753075ecba6377a0c
2019-05-13 11:00:55 -07:00
Luna Wei 5f027ec64d Rearrange order of manageChildren
Summary:
[General] [Fix] - Reorder operations of native view hierarchy

When we update the native view hierarchy in `manageChildren` we:
1. iterate through all views we plan to remove and remove them from the native hierarchy
2. iterate through all views we plan to add and add them to the native hierarchy
3. iterate through all views we plan to delete and delete them (remove them from memory)

This covers these cases:
a. A view is moved from one place to another -- handled by steps 1 & 2
b. A view is added -- handled by step 2
c. A view is deleted -- handled by step 1 & 3

> Note the difference between remove and delete

Everything above sounds fine! But...

The important bit:
**A view that is going to be deleted asynchronously (by a layout animation) is NOT removed in step 1. It is removed and deleted all in step 3.** See: https://fburl.com/ryxp626i

If the reader may recall we solved a similar problem in D14529038 where we introduced the `pendingIndicesToDelete` data structure to keep track of views that were marked for deletion but had not yet been deleted. An example of an order of operations that we would've solved with D14529038 is:

* we "delete" the view asynchronously (view A) in one operation
* we add a view B that shares a parent with view A in subsequent operation
* view A finally calls its callback after the animation is complete and removes itself from the native view hierarchy

A case that D14529038 would not fix:
1. we add a view B in one operation
2. we delete a view A in the same operation asynchronously because it's in a layout animation
3. ... etc.

What we must remember is that the index we use to add view B in step 1 is based on the indices assuming that all deletions are synchronous as this [comment notes](https://fburl.com/j9uillje): removals (both deletions and moveFroms) use the indices of the current order of the views and are assumed independent of each other. Whereas additions are indexed on the updated order (after deletions!)

This diff re-arranges the order in how we act upon the operations to update the native view hierarchy -- similar to how UIImplementation does its operations on the shadow tree here: https://fburl.com/j9uillje

By doing the removals and deletions first, we know that the addAt indices will be correct because either the view is removed from the native view hierarchy or `pendingIndicesToDelete` should be tracking an async delete already so the addAt index will be normalized

Reviewed By: mdvacca

Differential Revision: D15112664

fbshipit-source-id: 85d4b21211ac802183ca2f0fd28fc4437d312100
2019-05-06 08:35:40 -07:00
Estevão Lucas f70e58f355 - remove accessibilityComponentType and accessibilityTraits props (a11y) (#24344)
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
2019-04-15 10:53:50 -07:00
Joshua Gross bbd925cdd1 MountingManager can create views with props in one step instead of two
Summary: Create views with props in one call instead of two. Backwards-compatible.

Reviewed By: shergin

Differential Revision: D14846424

fbshipit-source-id: cb53225579089f7e51d4e9d1fc9fc2e331a994c1
2019-04-15 01:46:04 -07:00
landon e5d975b5c7 ignore dropView method when view is null (#24347)
Summary:
In #20288, we solved `com.facebook.react.uimanager.NativeViewHierarchyManager.dropView (NativeViewHierarchyManager.java:536)` by adding codes that prevent to run method on `view` object when `view.getId()` is null.

But if `view` is null, `view.getId()` can make error about NullPointerException. So, I think `dropView` method  needs to check if `view` is null.

If you need more information, Please read #24346.

[Android] [Fixed] - Ignore dropView method when view is null.
Pull Request resolved: https://github.com/facebook/react-native/pull/24347

Differential Revision: D14833822

Pulled By: cpojer

fbshipit-source-id: 88b6a05090ea8e8d6737c1f10b40e93649fab151
2019-04-08 12:44:48 -07:00
Pieter De Baets d9a82221a4 Back out "[react-native] Remove experimental gating for LayoutAnimation on Android"
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
2019-04-04 09:47:12 -07:00
Pieter De Baets 9895d01137 Remove experimental gating for LayoutAnimation on Android
Reviewed By: sahrens

Differential Revision: D14658087

fbshipit-source-id: 378ef4a5c5336d428b5045772d094a297b2767c7
2019-04-03 04:42:15 -07:00
Pieter De Baets f571c62ddf Implement completion callback for LayoutAnimation on Android
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
2019-04-03 04:42:14 -07:00
Pieter De Baets a333c2b202 Remove legacy AnimationManagerModule
Summary: This code was shipped as part of the initial open-source release but was never used.

Reviewed By: sahrens

Differential Revision: D14649389

fbshipit-source-id: 0c068ca69b91d275008f4a7af77a23a4f1470c18
2019-04-03 04:42:14 -07:00
Adam Comella a2285b1790 Android: Enable views to be nested within <Text> (#23195)
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
2019-04-01 19:55:23 -07:00
Luna Wei 20b4879dfd - LayoutAnimations cause invalid view operations
Summary:
[Android] [Fixed] - LayoutAnimations cause invalid view operations

The dating team has found a consistent repro where following an order of steps will get you the following exception:

https://lookaside.facebook.com/intern/pixelcloudnew/asset/?id=2113362972287761

The exception is due to the fact that the following operation
 `delete child tag 17 from parent tag 481 which is located at index 2`
cannot be performed because parent tag 481 only has 2 children.. and they also happen to not have the tag 17 as a child. Somehow the operations and the tree they act upon are out of sync.

RN receives operations from React via the native module `UIManagerModule`. The operations use tags (an identifier for a view) and indices to determine what view is updated. These operations (grouped together as a batch) are then passed to the UI thread where we perform them on the platform views.

LayoutAnimations are implemented per batch in RN. When LayoutAnimations are on, qualified view updates are animated. Because the delete operation is animated, RN doesn't remove the view from the platform view hierarchy immediately but asynchronously -- after the animation is complete. This is problematic for other view operations that rely on an index on where to insert or delete a view because during the creation of those operations, it was assumed all prior operations were performed synchronously.

This is what was occurring in the repro and there were silent view errors occurring before the exception was thrown.

This diff proposes a solution to track all pending delete operations across operations.
We introduce a map that is keyed on view tags and has a value of a sparse array that represents child index -> count of views that are pending deletion.
`{11: [0=1], 481: [2=1]}` where this would be interpreted as for index 0 under view tag 11, there is one async view deletion that has not completed and under tag 481 there is one async view deletion at index 2.

We use the map to adjust indices on add / delete operations. We also update the count when the deletion animation is complete and remove the tag from the map when it is deleted.

Regarding worst case sizing of the map => O(# of platform views rendered)

Reviewed By: mdvacca

Differential Revision: D14529038

fbshipit-source-id: 86d8982845e25a2b23d6d89ce27852fd77dbb060
2019-03-26 10:33:08 -07:00
Luna Wei c974b5e966 Remove SizeMonitoringFrameLayout
Summary: SizeMonitoringFrameLayout was used to set layout contraints on the root shadow node when the native view's size changes. Since then, we introduced ways for the root node to use proper layout contraints using the root view's measure specs, which provides more accurate constraints for the root node, so SizeMonitoringFrameLayout is no longer needed. This ends up making a lot of UIManagerModule's method signatures cleaner

Reviewed By: mdvacca

Differential Revision: D9565720

fbshipit-source-id: c569cd15991a09987cc01e89612dc9193ad99b45
2019-02-12 18:23:09 -08:00
Dulmandakh 4d95e85f64 remove unnecessary Android version checks (#23277)
Summary:
React Native's minSdkVersion is 16, or we support Android versions 16 (Jelly Bean) and above. But in the code we have many checks if Android is Jelly Bean or newer, which are unnecessary. This PR removes unnecessary Android version checks, also uses Android version names instead of numbers.

[Android] [Changes] - remove unnecessary Android version checks
Pull Request resolved: https://github.com/facebook/react-native/pull/23277

Differential Revision: D13955909

Pulled By: cpojer

fbshipit-source-id: 6b1caa5ef4fe42273d3c69a6617fff140a697b5c
2019-02-05 03:08:16 -08:00
Christoph Nakazawa 117dcd9c58 Fix error message in NativeViewHierarchy
Summary: It was pointed out to me in https://github.com/facebook/react-native/pull/22508 by raphael-liu that the error message refers to the wrong tag. I didn't merge the PR because I don't have good insight into the effects it could cause, but we should at least fix the error message.

Reviewed By: TheSavior

Differential Revision: D13564266

fbshipit-source-id: fa76f0888364df329d052dbcc2050f906d39dcef
2019-01-01 19:45:33 -08:00
David Vacca 39b6890346 Fix crash when removing root nodes
Summary: If a children of a root node is being removed and the Root Node is empty, it was likely already removed and cleaned previously, likely due to a race condition caused by RN's async nature. In those cases, let's avoid crashing the app and instead silently ignore the root view removal.

Reviewed By: fkgozali

Differential Revision: D13405817

fbshipit-source-id: 0179d10a88a2d19f1db5ea35b48cb83d9d7429a6
2018-12-11 03:28:04 -08:00
Andrew Chen (Eng) 83405ff316 Fix crash when releasing RN views
Summary: There are cases where we're trying to drop a view that is not associated with a ViewManager. This is likely caused by race conditions that can occur if we're dropping a view from JS (when it's no longer used) but at the same time dropping it from native (when layout animation ends, when the rootview gets unmounted). In either of those cases, it should be safe to ignore the drop operation because the view was likely dropped already.

Reviewed By: mdvacca

Differential Revision: D13036643

fbshipit-source-id: 260ffb56d32a0d670ad08f449b8df165b2533195
2018-11-14 10:51:15 -08:00
Andrew Chen (Eng) dc74975c5f Remove hard crash when root view is reused
Summary: Alternative to D10499861. If an app does not have an exception handler, context.handleException will still hard crash. Since this error is just saying that we're reusing an unexpected RootView, it might be safe to continue execution. Let's remove the exception for now while we investigate further

Reviewed By: mmmulani

Differential Revision: D10560413

fbshipit-source-id: 6c08a16cd250a019d2aef5afcaf3ba61534d29f7
2018-10-24 13:08:41 -07:00
Andrew Chen (Eng) f671f8868f Don't crash when ReactRootView's id is already set
Summary:
This is a bandaid fix to address a crash with a stack trace involving

```
com.facebook.react.uimanager.IllegalViewOperationException: Trying to add a root view with an explicit id already set. React Native uses the id field to track react tags and will overwrite this field. If that is fine, explicitly overwrite the id field to View.NO_ID before calling addRootView.
0+com.facebook.react.uimanager.NativeViewHierarchyManager.addRootViewGroup(NativeViewHierarchyManager.java:546) [inlined]
1+com.facebook.react.uimanager.NativeViewHierarchyManager.addRootView(NativeViewHierarchyManager.java:538) [inlined]
2+com.facebook.react.uimanager.UIViewOperationQueue.addRootView(UIViewOperationQueue.java:678) [inlined]
3+com.facebook.react.uimanager.UIImplementation.registerRootView(UIImplementation.java:216) [inlined]
4+com.facebook.react.uimanager.UIManagerModule.addRootView(UIManagerModule.java:355)
5+com.facebook.react.ReactInstanceManager.attachRootViewToInstance(ReactInstanceManager.java:1032)
6+com.facebook.react.ReactInstanceManager.attachRootView(ReactInstanceManager.java:726) [inlined]
7+com.facebook.react.ReactRootView.attachToReactInstanceManager(ReactRootView.java:524)
8+com.facebook.react.ReactRootView.startReactApplication(ReactRootView.java:377)
```

This crash seems to be happening because the root view's id is set to a non NO_ID value, but further up in the stack trace, UIManager.addRootView() is called which always sets the root view's id to NO_ID. It's not clear how this error is happening, but in this code path it's expected that addRootView should always ensure that the id is always reset. In order to avoid crashing for this, let's remove the exception and log the issue instead.

In the future, we should not be overloading the android view id field with these types of react native implementation details. The react tag should be stored as a view tag instead.

Reviewed By: mmmulani

Differential Revision: D10499861

fbshipit-source-id: 4dffedab4e7a34eee7f64bb43ec8209699521c73
2018-10-22 17:28:17 -07:00