Summary: It should be possible to use the latter without using the former.
Reviewed By: ashwinb
Differential Revision: D4321776
fbshipit-source-id: 935fbb3fdb47369e18992aca0497d312ad6023bc
Summary:
When using a TextInput with a custom font, the placeholder didn't use that font. This is because ReactTextInputManager didn't use ReactFontManager to create the TypeFace which handles custom fonts.
**Test plan**
Tested in UI explorer by reproducing the bug with and testing that the custom font gets applied properly after the fix.
``` js
<TextInput
placeholder="Hello"
style={{ fontFamily: 'notoserif' }}
/>
```
Closes https://github.com/facebook/react-native/pull/12000
Reviewed By: hramos
Differential Revision: D4443713
fbshipit-source-id: e92c9822d9226681d7b00126dad95e5534c0c46e
Summary:
Support `xhr.send(data)` for typed arrays.
**Test plan:** run UIExplorer example on iOS and Android.
Closes https://github.com/facebook/react-native/pull/11904
Differential Revision: D4425551
fbshipit-source-id: 065ab5873407a406ca4a831068ab138606c3361b
Summary:
For returnKeyType 'go', 'search' and 'send' Android will call
onEditorAction twice, once with IME_NULL and another time with the respective IME_ACTION.
This change makes sure to only fire one onSubmitEditing by always returning true in onEditorAction, which causes no subsequent events to be fired by android.
Fixes#10443
**Test plan**
1. Create view with TextInput having 'go', 'search' or 'send as `returnKeyType`
```javascript
<View>
<TextInput
returnKeyType='search'
onSubmitEditing={event => console.log('submit search')}></TextInput>
<TextInput
returnKeyType='go'
onSubmitEditing={event => console.log('submit go')}></TextInput>
<TextInput
returnKeyType='send'
onSubmitEditing={event => console.log('submit send')}></TextInput>
</View>
```
2. Input some text and click submit button in soft keyboard
3. See event fired only once instead of two times
Closes https://github.com/facebook/react-native/pull/11006
Differential Revision: D4439110
Pulled By: hramos
fbshipit-source-id: 5573b7f15f862b432600ddd3d61a0852ce51b2b3
Summary: Introduced IS_TESTING flag on Platform module for android as well. This is useful for testing environment.
Reviewed By: mmmulani
Differential Revision: D4429662
fbshipit-source-id: 33711d7fb5666f0bac8aee444b71261f7f12770f
Summary:
Many websites use domstorage and never think of its unavailability, which usually leads to a blank page on android and hard for developers to debug. I think it's better to enable domstorage by default, for convenience and consistency to iOS and PC.
Closes https://github.com/facebook/react-native/pull/11333
Differential Revision: D4437165
Pulled By: hramos
fbshipit-source-id: a00441cb5214cca27927471d3a33f030b9ff9b52
Summary: Since we don't support this, we should throw early. Also tries to improve the error message when adding a node that doesn't have a YogaNode to a node that can't measure itself.
Reviewed By: andreicoman11
Differential Revision: D4421542
fbshipit-source-id: e0be8cdd763091145e5e6af2db91515f4d236b37
Summary:
Fix#326. I'll open another PR once this one gets accepted to add support for `YGLayoutGetBorder` 👌
Closes https://github.com/facebook/yoga/pull/335
Reviewed By: gkassabli
Differential Revision: D4409399
Pulled By: emilsjolander
fbshipit-source-id: 8153f6701cab60b55a485f6d2e0b9f7767481090
Summary:
This is an attempt to fix the following startup exception that I get when running any of the example apps:
> Unrecognized type: interface com.facebook.react.bridge.Dynamic for method: com.facebook.react.uimanager.LayoutShadowNode#setFlexBasis
I really have no idea what I'm doing here, just trying to get UIExplorer to compile and run so I can test my upcoming PRs. ~~Unfortunately, this doesn't actually make the apps run, but at least it does get rid of the startup exception!~~ Edit: it works now.
**Test plan:** Run Movies and UIExplorer example app
Closes https://github.com/facebook/react-native/pull/11896
Differential Revision: D4418927
Pulled By: mkonicek
fbshipit-source-id: 34788b790b6bfc46ff39a0d9ca1698a5c20662e1
Summary: The include was missing in 87c6bcb65d (D4405355)
Reviewed By: ericnakagawa
Differential Revision: D4417874
fbshipit-source-id: 164a44e08c9da0931b49c90d01eb4396225e27d1
Summary:
== What ==
Changing the `JSBundleLoader` API, to remove `String getSourceUrl()`, instead `JSBundleLoader.loadScript` now returns the source URL it loaded.
This change has a knock-on effect: We can no longer populate `SourceCodeModule` when we construct it, because at that time, we do not know the source URL.
In order to solve this I have made the following changes:
- Added `CatalystInstance.getSourceURL()`, to return the source URL from the instance after the JS Bundle has been loaded, or `null` otherwise.
- Removed `ReactInstanceManager.getSourceUrl()`, because its only purpose was to populate `SourceCodeModule`.
- Also removed `ReactInstanceManager.getJSBundleFile()` because it was only being used in a test confirming that the `ReactInstanceManager` knew its bundle file as soon as it was constructed, which is no longer necessarily true.
- Initialise `SourceCodeModule` with the `ReactContext` instance it belongs to.
- Override `NativeModule.initialize()` in `SourceCodeModule` to fetch the source URL. When the `SourceCodeModule` is constructed, the context does not have a properly initialised `CatalystInstance`, but by the time we call initialise on it, the `ReactContext` has a `CatalystInstance` and that in turn has a source URL.
== Why ==
The reason for this change is that it allows us to add implementations of `JSBundleLoader`, that cannot determine their source URL until after having performed a load successfully. In particular I plan to introduce `FallbackJSBundleLoader` which will try to load from multiple sources in sequence stopping after the first successful load. As load failures could happen for a variety of reasons, we can't know what the true source URL is without performing the load.
Reviewed By: javache
Differential Revision: D4398956
fbshipit-source-id: 51ff4e289c8723e9d242f23267181c775a6abe6f
Summary:
Currently, < WebView > allows you to pass JS to execute within the view. This works great, but there currently is not a way to execute JS after the page is loaded. We needed this for our app.
We noticed that the WebView had messaging support added (see #9762) . Initially, this seemed like more than enough functionality for our use case - just write a function that's injected on initial load that accepts a message with JS, and `eval()` it. However, this broke once we realized that Content Security Policy can block the use of eval on pages. The native methods iOS provide to inject JS allow you to inject JS without CSP interfering. So, we just wrapped the native methods on iOS (and later Android) and it worked for our use case. The method injectJavaScript was born.
Now, after I wrote this code, I realized that #8798 exists and hadn't been merged because of a lack of tests. I commend what was done in #8798 as it sorely solves a problem (injecting JS after the initial load) and has more features than what I'
Closes https://github.com/facebook/react-native/pull/11358
Differential Revision: D4390425
fbshipit-source-id: 02813127f8cf60fd84229cb26eeea7f8922d03b3
Summary:
Added baseline support (see #132)
You have the ability for a custom baseline function (```float(*YGBaselineFunc)(YGNodeRef node);```) to return whatever baseline you want.
Closes https://github.com/facebook/yoga/pull/317
Reviewed By: splhack
Differential Revision: D4385061
Pulled By: emilsjolander
fbshipit-source-id: cb8a59a09237c840fa3e21753ab68239997dab0c
Summary: Explicitly set default scrollbarstyle value. Previously this style was implicitly used as a side effect of how we set padding on the Scrollview. This instead makes that behavior explicit.
Reviewed By: astreet
Differential Revision: D4386861
fbshipit-source-id: 362d82136a12b75fb81287ac0d0fd58f2ee297fb
Summary:
Adds the feature to use percentage as a value unit.
You can use the function ```YGPx(float)``` and ```YGPercent(float)``` for convenience.
I did some benchmarks:
```
Without Percentage Feature - Release x86:
Stack with flex: median: 0.000000 ms, stddev: 0.146683 ms
Align stretch in undefined axis: median: 0.000000 ms, stddev: 0.136525 ms
Nested flex: median: 0.000000 ms, stddev: 0.490101 ms
Huge nested layout: median: 23.000000 ms, stddev: 0.928291 ms
Stack with flex: median: 0.000000 ms, stddev: 0.170587 ms
Align stretch in undefined axis: median: 0.000000 ms, stddev: 0.143384 ms
Nested flex: median: 0.000000 ms, stddev: 0.477791 ms
Huge nested layout: median: 22.000000 ms, stddev: 2.129779 ms
With Percentage Feature - Release x86:
Stack with flex: median: 0.000000 ms, stddev: 0.132951 ms
Align stretch in undefined axis: median: 0.000000 ms, stddev: 0.136525 ms
Nested flex: median: 0.000000 ms, stddev: 0.489570 ms
Huge nested layout: median: 21.000000 ms, stddev: 1.390476 ms
Closes https://github.com/facebook/yoga/pull/258
Reviewed By: dshahidehpour
Differential Revision: D4361945
Pulled By: emilsjolander
fbshipit-source-id: a8f5bc63ad352eb9410d792729e56664468cd76a
Summary: Update the Android RecyclerView, support v4, and annotation libraries to 23.4.0.
Differential Revision: D4345649
fbshipit-source-id: 859c6555bc79358b1c8ffed0629cdf0e83408a00
Summary:
Before this patch, each Node would always generate a node
region, representing the bounds of this particular Node. This set of Nodes was
used when handling touch to figure out whether we should intercept touch (i.e.
a flat Node is catching the draw), or let Android handle touch (i.e. a Node
mounted to a View will handle the touch).
This patch modifies the list of NodeRegions to exclude any Nodes that draw
nothing at all. These Nodes, having no draw output, are effectively layout
containers used to group items, so they shouldn't handle touch.
Reviewed By: sriramramani
Differential Revision: D4369484
fbshipit-source-id: 71b41611873580631f1639f368fa8d971995874f
Summary:
Virtual nodes do not have backing Yoga nodes, so measure
their first non-virtual parent instead of measuring them.
Reviewed By: sriramramani
Differential Revision: D4360540
fbshipit-source-id: 505d35fec74dddf67b002d29268acc29d2651b13
Summary:
Like its non-Nodes counterpart, FlatARTSurfaceViewShadowNode
should redraw when extra updates are collected.
Reviewed By: sriramramani
Differential Revision: D4355702
fbshipit-source-id: 6e7b90360958481f5bef9b806eca9fa888cb6a32
Summary:
Add a README file explaining the purpose of Nodes and how to
enable it within an app.
Reviewed By: JoelMarcey, lacker
Differential Revision: D4349517
fbshipit-source-id: ec26ebb37039e7c23ecd2cf4b482fa21ca8beeda
Summary:
drawRect was sometimes being called with NaN values, which caused
very strange ui behavior on some devices. This patch fixes the problem by
ensuring that we use a default value when the value is NaN.
Reviewed By: AaaChiuuu
Differential Revision: D4338453
Summary: Update package name of java code to refer to yoga instead of csslayout. Still need to change the name of the folder where this code resides but that requires update github sync scripts etc so it is safer and easier to split these diffs apart.
Differential Revision: D4271420
Summary: Remove deprecated java code and make use of CSSEdge instead of the now removed Spacing class.
Reviewed By: AaaChiuuu
Differential Revision: D4233198
Summary:
@public
Virtual shadow nodes (e.g. text) don't use CSSNodes so we don't need to create them. This shows large savings in CSSNodes allocated, depending on the app.
This could be breaking if:
- You have virtual nodes that still set and get CSS properties. The setters now no-op for virtual nodes (I unfortunately couldn't remove them completely -- see the comment on LayoutShadowNode), but the getters will NPE. If you see these NPE's, you should almost definitely be using your own datastructure instead of a CSSNode as virtual nodes will not participate in the layout process (and the CSSNode is then behaving just as a POJO for you).
I do not anticipate this to be breaking for anyone, but am including breaking in the commit message since this is a change in API contract.
Reviewed By: emilsjolander
Differential Revision: D4220204
Summary:
@public
Adds a pool to recycle CSSNodes within UIManager. A follow-up diff will hook this up to a memory pressure listener to drop the pool on memory pressure.
Reviewed By: emilsjolander
Differential Revision: D4189532
Summary: setPadding already calls dirty, and we should only be calling dirty on nodes that have measure functions.
Reviewed By: emilsjolander
Differential Revision: D4205148
Summary:
@public
This diff makes it so ReactShadowNode holds a CSSNode instead of extending one. This will enable us to pool and re-use CSSNodes and will allow us to keep from breaking the CSSNode api assumption that nodes that have measure functions don't have children (right now, text nodes have measure functions, but they also have raw text children).
BREAKING
This diff makes ReactShadowNode no longer extend CSSNodeDEPRECATED. If you have code that depended on that, e.g. via instanceof checks, that will no longer work as expected. Subclasses that override getChildAt/addChildAt/etc will need to update your method signatures. There should be no runtime behavior changes.
Reviewed By: emilsjolander
Differential Revision: D4153818
Summary: This is an API breaking change done to allow us to avoid an allocation during measurement. Instead we do the same trick as is done when passing measure results to C, we path them into a long.
Reviewed By: splhack
Differential Revision: D4081037
Summary:
In Nodes, we sometimes get crashes when we drop an already dropped
view. For now, this catches the exception to allow things to be handled
gracefully (until we can identify the actual root cause). #accept2ship
Reviewed By: sriramramani
Differential Revision: D4059333
Summary: The current implementation was made out of simplicity and to keep the same API as before. Now that the java version of csslayout is deprecated it is time to change the API to make the calls more efficient for the JNI version. This diff with reduce allocations as well as reduce the number of JNI calls done.
Differential Revision: D4050773
Summary:
From task:
in Nodes today, styles (dashed and dotted) only work on borders if the view has rounded corners. this is incorrect as they should work even when we're drawing rectangular borders.
Looking at the current non-nodes code (https://fburl.com/474407319) we can see that whenever there is a pathstyle effect the non-nodes code treats the border as if it was rounded (note that mBorderStyle == null || mBorderStyle == BorderStyle.SOLID means NO path effect is applied).
To bring the Nodes code in line with the non-Nodes code we can simply do the same thing: if there is a path style effect draw it as if it was rounded.
Reviewed By: ahmedre
Differential Revision: D4054722
Summary:
From task:
In some cases, however, drawPath is the more correct thing to do, and this ticket is one such example - if we draw the left border and top border with different colors, Nodes draws rectangularly, whereas non-Nodes makes the edges triangular.
To solve the issue in Nodes, we use drawPath instead of drawRect only when necessary (borders intersect with different colors).
Reviewed By: ahmedre
Differential Revision: D4048685
Summary:
Implemented 2 TODOs from ReactART for Android:
- TODO(7255985): Use TextureView and pass Surface from the view to draw on it asynchronously instead of passing the bitmap (which is inefficient especially in terms of memory usage)
- TODO(6352067): Support dashes in ARTShape
We use ReactNativeART in our Android project.
1. Our app crashes sometimes on large screen smartphones with OutOfMemoryError. Crashes happen in ARTSurfaceShadowNode where TODO(7255985) was suggested in a comment in order to use memory more efficiently.
2. We needed dashes for drawing on ARTSurface.
**Test plan (required)**
I attach a screenshot of our app which shows dashed-lines and two ARTSurfaces on top of each other rendering exactly the same as in the pervious implementation of ARTSurface.
Closes https://github.com/facebook/react-native/pull/9486
Differential Revision: D4021303
Pulled By: foghina
Summary:
In Nodes, we added logic when we dropped a view to also pass the
parent, so we could tell the parent that \"hey, this view is now dropped\"
so that it can be released. While this is fine, there are some crashes due
to the fact that the root node is not being found when we drop the child.
I've spent a lot of time thinking about how this could happen, and the only
plausible explanation I can come up with is that we first detach all views
from the root, then drop the root, and then drop one of the children that
was detached. I can't think of a good way why this would happen.
In the interest of protecting from this crash, this patch adds a check as to
whether or not the id of the parent is that of a root id, and, if so, it
doesn't run the logic that tells this view that this view was removed.
This should be safe, because the root most view should not have clipping
enabled (since it's a vanilla view group as opposed to a scrolling view).
Reviewed By: sriramramani
Differential Revision: D3991682
Summary:
This way `UIImplementation` can hold on to it and use it outside of calls from the `UIManagerModule`.
@public
Reviewed By: lexs
Differential Revision: D3899774
Summary:
@public
The hack for the status bar height is not necessary any longer, so we can remove
all code related to it
Reviewed By: lexs
Differential Revision: D3943770
Summary:
Modals were doing nothing (and sometimes crashing) when they were
being closed. The reason for this was due to the fact that the parent being
removed was not necessarily the view's parent. Consequently, trying to inform
said parent that its child was removed failed, because said parent wasn't a
view, and therefore had no record in mViewsToTags.
Reviewed By: sriramramani
Differential Revision: D3928850
Summary:
@public
This fixes measuring of items in the main axis of a container. Previously items were in a lot of cases measured with UNSPECIFIED instead of AT_MOST. This was to support scrolling containers. The correct way to handle scrolling containers is to instead provide them with their own overflow value to activate this behavior. This is also similar to how the web works.
This is a breaking change. Most of your layouts will continue to function as before however some of them might not. Typically this is due to having a `flex: 1` style where it is currently a no-op due to being measured with an undefined size but after this change it may collapse your component to take zero size due to the implicit `flexBasis: 0` now being correctly treated. Removing the bad `flex: 1` style or changing it to `flexGrow: 1` should solve most if not all layout issues your see after this diff.
Reviewed By: majak
Differential Revision: D3876927
Summary:
With Nodes, we want to support recursive clipping of subviews.
Without this, surfaces like Marketplace won't properly handle subviews.
Reviewed By: sriramramani
Differential Revision: D3904721
Summary:
This fixes a crash for the case when we try to drop a view that has already been dropped.
**The Problem**
We got reports of a crash (t12912526) that occurs when the resolveViewManager method can't resolve a ViewManager for a View being dropped.
Investigating this, one thing in common between all the stack traces for this is that dropView is called from line 210 of FlatNativeViewHierarchyManager. This part of the code is specifically the part we added to remove strong references to any clipped children (from views that have subview clipping enabled).
So this is a problem specifically with Nodes and clipSubviews, which brings up some questions:
**when can this happen?**
The only situation this can possibly happen is when we drop a child (which is clipped) followed by dropping its parent in the same cycle. Consider a tree where each view only has one child, such as: A - B - C - D. This crash would happen if D is clipped, and we removed it, followed by removing any of its parents in the same frame.
**if the removes happen in different frames, does this bug occur?**
No - the reason is that before we execute the DropView operations, we run through StateBuilder, which traverses the shadow tree and marks updates, thus removing the view going away (such that the delete in the next frame doesn't try to re-delete it).
So why doesn't this happen when we're dropping in the same frame? The reason is that manageChildren (where this all starts) asks to remove some views. We handle this by removing said Nodes and their children from the shadow tree. Consequently, when StateBuilder iterates over the shadow tree, it can't do the right thing because said nodes no longer exist.
As a more concrete example, consider A - B - C - D again, and consider that both D and B are removed. StateBuilder only sees A, and realizes that it now has 0 children (whereas before it has 1), so it removes B from its children. However, this process isn't recursive, so C never gets cleaned up.
**why doesn't this happen with Nodes without clipping containers?**
The answer to this is that NativeViewHierarchyManager's dropView method checks the existance of each child before deeply dropping that child and its subtree. So in this case, we drop D and all its children, and when we come to drop B, we try to drop C (which exists) and then its children (D, which doesn't exist because we already dropped it, so we ignore it).
**why doesn't this happen with non-Nodes?**
The reason is that non-Nodes handles removes differently - every remove is enqueued in a call to NativeViewHierarchy's manageChildren, which explicitly asks the parent to remove said child. Consequently, we never try to remove a child that is already removed.
**Fix**
The initial fix was to check whether or not the view exists, but this updated patch just does the right thing at drop time - i.e. whenever a view is dropped, we notify the parent of this fact so that it can clear the reference from clipped views.
**One last Note**
There are two reasons for switching `super.dropView` to `dropView` - first, the comment is only partially correct - calling `super.dropView` will avoid looking at clipped children (as an aside, that could cause a leak in the case of nested clipping subviews), but will look at clipped grandchildren, because of the super class's iteration across the set of children.
Reviewed By: astreet
Differential Revision: D3815485
Summary:
This should probably be two separate diffs, sorry. It takes forever to test these things on fb4a though.
The nodes GK was turned up in fb4a, so I had to make a few changes to make the existing integration work.
Reviewed By: lexs, emilsjolander
Differential Revision: D3863226
Summary:
@public
Introduce `overflow:scroll` so that scrolling can be implemented without the current overflow:visible hackiness. Currently we use AT_MOST to measure in the cross axis but not in the main axis. This was done to enable scrolling containers where children are not constraint in the main axis by their parent. This caused problems for non-scrolling containers though as it meant that their children cannot be measured correctly in the main axis. Introducing `overflow:scroll` fixes this.
Reviewed By: astreet
Differential Revision: D3855801
Summary:
@public
Introduce `overflow:scroll` so that scrolling can be implemented without the current overflow:visible hackiness. Currently we use AT_MOST to measure in the cross axis but not in the main axis. This was done to enable scrolling containers where children are not constraint in the main axis by their parent. This caused problems for non-scrolling containers though as it meant that their children cannot be measured correctly in the main axis. Introducing `overflow:scroll` fixes this.
Reviewed By: astreet
Differential Revision: D3855801
Summary:
@public
This is to be able to depend on ReactClippingViewGroup from BaseViewManager. Devs using ReactClippingViewGroup may need to update their imports when updating past this commit.
Reviewed By: lexs
Differential Revision: D3835328
Summary:
We were always getting LEFT explicitly, and, due to RTL support, we
should be asking for START instead.
Reviewed By: sriramramani
Differential Revision: D3836816
Summary:
@public
Setting the line height with the help of Android-provided StaticLayout is incorrect. A
simple example app will display the following when `setLineSpacing(50.f, 0.f)`
is set: {F62987699}. You'll notice that the height of the first line is a few
pixels shorter than the other lines.
So we use a custom LineHeightSpan instead, which needs to be applied to the text
itself, and no height-related attributes need to be set on the TextView itself.
Reviewed By: lexs
Differential Revision: D3841658
Summary:
Fix TextInput padding on Nodes. We used to not call super and used
to manually do the setting of padding. This stops us from running the same
logic that non-Nodes runs (we bypassed it), so this fixes it.
Reviewed By: astreet
Differential Revision: D3825227
Summary:
Due to the RTL implementation, the ViewProps spacing array has START
and END, whereas Nodes should deal with RIGHT and LEFT directly (just like
non-Nodes does). This is the same implementation in use by non-Nodes.
Reviewed By: astreet
Differential Revision: D3809028
Summary:
Nodes historically had two image implementations -
DrawImageWithDrawee and DrawImageWithPipeline. The drawee implementation
was the default (per request of the Fresco team). At this point, there is
no point of having two (especially since updates to one need to be made to
the other), so this patch removes pipeline.
Reviewed By: sriramramani
Differential Revision: D3755523
Summary:
Nodes would typically clip its images, and then Fresco would then
re-clip as part of ScaleTypeDrawable - in addition to being unnecessary,
it's also incorrect, beacuse it causes the image to be smaller than it
should be.
Reviewed By: sriramramani
Differential Revision: D3754778
Summary:
@public
Setting the line height with the help of Android-provided StaticLayout is incorrect. A
simple example app will display the following when `setLineSpacing(50.f, 0.f)`
is set: {F62987699}. You'll notice that the height of the first line is a few
pixels shorter than the other lines.
So we use a custom LineHeightSpan instead, which needs to be applied to the text
itself, and no height-related attributes need to be set on the TextView itself.
Reviewed By: lexs
Differential Revision: D3751097
Summary:
@public
Setting the line height with the help of Android-provided StaticLayout is incorrect. A
simple example app will display the following when `setLineSpacing(50.f, 0.f)`
is set: {F62987699}. You'll notice that the height of the first line is a few
pixels shorter than the other lines.
So we use a custom LineHeightSpan instead, which needs to be applied to the text
itself, and no height-related attributes need to be set on the TextView itself.
Reviewed By: lexs
Differential Revision: D3751097
Summary:
This is just a minor cleanup, use constants for the LEFT
and RIGHT alignments, since they are hide.
Reviewed By: sriramramani
Differential Revision: D3746019
Summary: Adds a flag that can be set in FlatViewGroup to see known performance issues in Nodes objects. This is mostly useful in internal development of Nodes, and will be a dead code path when not set.
Reviewed By: ahmedre
Differential Revision: D3732675
Summary: Node region bounds are assumed to equal the underlying node bounds. In the case of hit slop, these need to be abstracted.
Reviewed By: ahmedre
Differential Revision: D3713430
Summary:
This is minor, but for our use case a SparseArray is going to be faster as long as we have less than 10,000 clipped subviews, and will also use much less memory.
Faster because of the boxing, unboxing and hash caching; less memory as it is two arrays instead of the object overhead of the HashMap.
Reviewed By: ahmedre
Differential Revision: D3704326
Summary: Previously, the first time we collected a draw view, we would make a clone, even though the draw view had never been mutated. This refactors draw view to avoid this extra allocate.
Reviewed By: ahmedre
Differential Revision: D3719056
Summary:
In Nodes, there were certain cases where text wasn't drawn due to an
optimization that skipped measuring because the size was already known.
Reviewed By: emilsjolander
Differential Revision: D3713841
Summary:
For Nodes that don't mount to views, measureLayout wasn't working
because our calls for getting the width and height would return the view delta
bounds, which won't exist for Nodes. #accept2ship
Differential Revision: D3707880
Summary:
Nodes currently doesn't ask Fresco to resize images, but this is
potentially problematic (ex having a camera photo of 6000x1500 causes a crash
due to the massive size).
Differential Revision: D3687944
Summary: Adds support for horizontal clipping, though the FlatViewGroup needs to be made aware still of which it is.
Reviewed By: ahmedre
Differential Revision: D3673501
Summary: This optimizes node region searches in clipping cases, and does position calculation for drawCommands off of the UI thread.
Reviewed By: ahmedre
Differential Revision: D3665301
Summary: Add jni bindings for csslayout. First step in many of removing LayoutEngine.java and performing all layout in native.
Reviewed By: lucasr
Differential Revision: D3648793
Summary:
Use ImageRequestBuilder directly in Nodes, just like we do for
non-Nodes.
Reviewed By: sriramramani
Differential Revision:
D3660610
Ninja: Sandcastle is broken. 25 denizens of the Facebook republic are affected by this unrelated issue today.
Summary: Add directional aware clipping to DrawCommandManager. Currently not attached to FlatViewGroup logic, with the plan to keep this unattached until we are clipping the way we want to in the final state.
Reviewed By: ahmedre
Differential Revision: D3622253
Summary: @public Make UIOperation public so that custom implementations can expose instances of it.
Reviewed By: ahmedre
Differential Revision: D3618197
Summary:
Support rounded clipping in Nodes. Before, if a view had
a radius and had overflow of hidden, its children could still draw
outside of it (specifically, in the area between the rounded rect
and square rect) - this is due to the fact that clipping is, by
default, rectangular. This patch supports this type of rounded
clipping.
Differential Revision: D3634861
Summary: Previously, we had no information about the positioning of the view until after we had attached it. We have the position information attached to the shadow node, but this attaches it to the DrawView as well. It also removes the need for AbstractClippingDrawCommand.
Reviewed By: ahmedre
Differential Revision: D3609092
Summary: @public Change the textalign setter to support RTL. In order to support text alignment according to layout style, move the textalign setter bridge function from ReactTextViewManager.java to ReactTextShadowNode.java and calculate it correctly on RCTTextUpdate.
Reviewed By: dmmiller
Differential Revision: D3597494
Summary:
This patch fixes measureInWindow for Nodes backed by Views.
Whereas the intention was to call the super implementation when we have a
Node backed by a View, we instead called the super implementation of
measure, which doesn't measure relative to window.
Differential Revision: D3607890
Summary:
Currently we have race conditions in DrawView related to isViewGroupClipped, where we create a copy of the DrawView before we update the clipping state, and think the view is unclipped in the next iteration.
Also we are sometimes creating a DrawView with a reactTag of 0.
This fixes both, and is part of the upcoming DrawView bounds change, but is a separate issue that is live in current source.
Reviewed By: ahmedre
Differential Revision: D3598499
Summary:
@public
This is pure cleanup so that we can make sure that all events are living in the same time space (currently nano seconds).
Reviewed By: foghina
Differential Revision: D3593884
Summary:
View the comment thread for discussion:
https://www.facebook.com/groups/1505872839725322/permalink/1630102823968989
Our current behaviour of add then immediately remove if a view is clipped pretty much guarantees that we kill network requests for images in feed. We have a better fix for that in the pipeline, but this is a low risk fix in the meantime.
Reviewed By: ahmedre
Differential Revision: D3597785
Summary:
This PR was split from a commit originally in #8619. /cc dmmiller
When an inline image was larger than the specified line height,
the image would be clipped. This changes the behavior so
that the line height is changed to make room for the inline
image. This is consistent with the behavior of RN for iOS.
Here's how the change works.
ReactTextView now receives its line height from the layout thread
rather than directly from JavaScript.
The reason is that the layout thread may pick a different line height.
In the case that the tallest inline image is larger than the line
height supplied by JavaScript, we want to use that image's height as
the line height rather than the supplied line height.
Also fixed a bug where the image, which is supposed to be baseline
aligned, would be positioned at the wrong y location. To fix this,
we use `y` (the baseline) in the `draw` method rather than trying
to calculate the baseline from `bottom`. For more information
see https://code.google.com/p/andro
Closes https://github.com/facebook/react-native/pull/8907
Differential Revision: D3592781
Pulled By: dmmiller
Summary:
DrawImageWithDrawee has caused NPEs when using Nodes in various cases
in RNFeed. This patch explicitly throws a RuntimeException, so that we can
debug as to whether this is coming from bad sources or a bad size for the
image.
Differential Revision: D3574998
Summary: This is the most straightforward fix for the double detach issue. If a view is not attached, then addViewInLayout never propagates onAttach, and adding through attachViewToParent is a no op. We could hack something in to attach clipped FlatViewGroups in onClippingRect, but any other view that relies on onAttachedToWindow will have similar issues.
Reviewed By: ahmedre
Differential Revision: D3560565
Summary: Since Nodes' manageChildren doesn't enqueue the child updates immediately, commands were being directed to non-updated views. Previously we applied updates for the shadow node before dispatching the command, but we can instead wait to fire commands until after we update the view hierarchy.
Reviewed By: ahmedre
Differential Revision: D3568541
Summary: Supports show layout bounds either by override within FlatViewGroup, or if show layout bounds is set in settings. Currently requires app restart to disable.
Reviewed By: ahmedre
Differential Revision: D3553669
Summary: Fixes needing to specify exact height of react views in Mason. Uses a ViewTreeObserver to delay draw until we have correct bounds.
Reviewed By: sriramramani
Differential Revision: D3527122
Summary:
Nodes wasn't supporting text decorations to the line (strike through
and underline). This patch implements that.
Differential Revision: D3512711
Summary: We do want to only apply updates when a view previously wasn't mounted and didn't have a backing view created. Previously we were applying updates to the view regardless of the mount state, which resulted in positioning bugs. Rather than revert, I cleaned up the code Ahmed fixed, since didUpdate || ensureBackingViewIsCreated() was both a bug and obscure, as the two should have a swapped order.
Reviewed By: sriramramani
Differential Revision: D3538734
Summary:
@public
Text was not correctly respecting padding. We would account for it when measuring, but then not actually apply the padding to the text. This adds support for proper padding
Reviewed By: andreicoman11
Differential Revision: D3516692
Summary:
Previously, to fix the issue of commands happening before the Views
were made and attached to the hierarchy, a check was added to see if a node
had not been mounted to a View, to update its hierarchy. In reality, we need
to do this irrespective, since a node could be mounted to a View, but its
children may not yet be attached, for example. Note that if there is nothing
to be done, this won't do extra work (i.e. applyUpdates recursively goes
through the tree from the node on which we did the operation to apply updates,
but if there are no updates, we stop traversing that praticular subtree).
Reviewed By: sriramramani
Differential Revision: D3511462
Summary:
The TextInput spannables are being set wrong by Nodes. Consequently,
when you hit space after a word, anything you type is highlighted, though it
shouldn't be.
Differential Revision: D3507516
Summary:
The results from measureInWindow were always wrong the first time it
was called. This was due to the fact that the view in question was not
actually a view yet, so the results were incorrect. This patch uses the
existing measure functionality (which can measure virtual nodes) to measure
the view, while modifying it to properly get the results relative to the
window instead of relative to the root view.
Reviewed By: sriramramani
Differential Revision: D3501544
Summary:
Depends on D3120798
Depends on D3120631
Enables D3120631 for nodes. This implementation seems to work but let me know if I'm doing something really stupid.
Reviewed By: ahmedre
Differential Revision: D3120814
Summary:
Modals were broken in Nodes, because the custom measurement logic for
all the children of the ReactModalShadowNode was not being applied (because we
wrapped it in a NativeViewWrapper). This change adds a custom flat node type
for modals.
Differential Revision: D3499557
Summary:
Fix DrawImageWithPipeline's code for checking whether or not an image
request exists or not to be the same as DrawImageWithDrawee's.
Differential Revision: D3489532
Summary:
In manageChildren, we were assuming that the indices that
were passed in to be removed were sorted, however, they weren't.
This patch sorts the children to be removed. Note that it doesn't
explicitly sort move, since these are sorted by the MoveProxy class.
Reviewed By: astreet
Differential Revision: D3474639
Summary:
Groups encountered a pretty major crash where, in many cases,
we would find that DrawCommands and Views were out of sync. This
turns out to be due to the fact that when we drop views from the
root view, we remove each child using removeChildAt (which ultimately
causes an invalidate and redraw). If this happens for a
FlatViewGroup, this causes issues where the Views are all removed,
but there are some DrawCommands (potentially DrawViews) that aren't
removed, hence them going out of sync.
Reviewed By: astreet
Differential Revision: D3473916
Summary: Currently only FlatViewGroup children were clipped, rather than all offscreen Android views.
Reviewed By: ahmedre
Differential Revision: D3462002
Summary:
During the patch for fixing the order of UI operations, we apply
updates to any node receiving a ViewManager command in order to ensure that
nodes that were not yet mounted to a View and not yet attached to their parent
would be properly able to receive the event. However, if a node is already a
view, calling the update could cause unwanted things to happen (for example,
the View's bounds changing improperly), because we're only traversing that
node of the tree and down (instead of the entire tree). This fixes the issue
by only applying updates to the node if the view mount state has changed.
Reviewed By: sriramramani
Differential Revision: D3448356
Summary:
A Layout's text can either be an Ellipsizer or a SpannedEllipsizer.
SpannedEllipsizer implements Spannable, but Ellipsizer doesn't. We were
casting the Layout's text directly to a Spanned without first checking as to
whether or not it was actually a Spanned.
Reviewed By: sriramramani
Differential Revision: D3435075
Summary:
The nodes version of D3364550. The only difference is that here we
don't get `onSizeChanged` but `onBoundsChanged`, and we need to compute the
height/width of the target image from those bounds. ahmedre please let me know
if any of these assumptions are in any way incorrect.
Reviewed By: ahmedre
Differential Revision: D3424843
Summary:
This is needed for the upcoming loading from multiple sources (D3364550 for the non-nodes version) and cache interrogation (D3392751 for non-nodes version).
This postpones creating the DraweeRequestHelper until the image size is known, which in the nodes universe is when `onBoundsChanged` is called.
Reviewed By: foghina, ahmedre
Differential Revision: D3413467
Summary:
Fix touch inspector when using Nodes by implementing custom logic.
This logic now takes into account that non-View nodes need to be clickable.
Reviewed By: astreet
Differential Revision: D3433927
Summary:
Made some improvements to RCTText based on some of our learnings from components for android. This now resembles diffusion/FBS/browse/master/fbandroid/java/com/facebook/components/widget/TextSpec.java
Things that have improved:
- Calculation of text width is now faster (we noticed in components that .getWith() on the layout is all that is needed and it is much faster)
- Use text layout builder to abstract away a lot of the low level details of static / boring layouts and text measurements
- Handle MeasureMode correctly, previously AT_MOST was not supported.
- Better handling of RTL text by using TextLayoutBuilder where I made changes to support RTL text in components. Specifically RTL text measured with UNSPECIFIED or AT_MOST.
- There was an incorrect assumption being made that when measure() was not called the text had to be boring. This is incorrect, Arabic text is never boring for example. Also multiline text is not boring either and may have exact sizing.
Reviewed By: ahmedre
Differential Revision: D3374752
Summary:
The dispatchViewManager command should, according to the spec, only
be executed after children are added. On Nodes, however, due to the fact that
the Views in question may not have been created until the call to the command
occurred, the dispatchViewManagerCommand may occur too early. Consequently,
ensure that we apply any state updates to the Node represented by that
reactTag before we enqueue the view manager command (this will ensure that
views are properly added to the parent, etc before sending the command).
Reviewed By: astreet
Differential Revision: D3428855
Summary:
@public
This adds support for specifying multiple sources for an image component, so that native can choose the best one based on the flexbox-computed size of the image.
The API is as follows: the image component receives in the `source` prop an array of objects of the type `{uri, width, height}`. On the native side, the native component will wait for the layout pass to receive the width and height of the image, and then parse the array to find the best fitting one. For now, this does not support local resources, but it will be added soon.
To see how this works and play with it, there's an example called `MultipleSourcesExample` under `ImageExample` In UIExplorer.
Reviewed By: foghina
Differential Revision: D3364550
Summary:
Historically, removeClippedSubviews for Nodes would not clip views
that overflowed their parent. This patch changes that, so that Nodes can
properly clip views when they are off screen (even if they have descendants
that overflow the bounds of their parent).
This is done by calculating a set of offsets from the actual width and
height of the view, and using those in the clipping calculations.
Reviewed By: sriramramani
Differential Revision: D3409859
Summary:
As an optimization, for something like a ScrollView which contains
a FlatViewGroup containing posts, make sure that each post is explicitly
mounted to a View. This may help improve performance, especially when said
Views are otherwise inlined as DrawCommands instead of actual Views.
Reviewed By: astreet
Differential Revision: D3161232
Summary:
The removeClippedSubviews optimization often detaches views while
maintaining strong references to them (so they can be attached again later
on). However, when removing the parent view, any detached views end up not
being cleaned up or removed, thus leaking memory. This fixes this by
explicitly dropping detached views when the parent is removed.
Reviewed By: astreet
Differential Revision: D3337513
Summary:
Text in Nodes is squashed into a single DrawCommand for drawing a
Boring or StaticLayout. Touch is handled using a TextNodeRegion subclass of
NodeRegion that knows how to identify pieces of text inside of the DrawCommand
based on spans in the text. However, we only use a TextNodeRegion on the
second call for updateNodeRegion for an RCTText. If there is only one call,
the NodeRegion will just be a normal one. This patch ensures that the
NodeRegion for an RCTText is always a TextNodeRegion, allowing for null
Layouts that are set when the DrawCommand is made.
Reviewed By: astreet
Differential Revision: D3291682
Summary:
As of D3235050, Nodes supports the optimization of removing clipped
subviews from the hierarchy. However, because Nodes supports overflow:visible,
this could cause issues when DrawCommands overflow the bounds of their parent
container. This patch fixes this by not clipping any overflowing Nodes.
Reviewed By: astreet
Differential Revision: D3235072
Summary:
RN has an optimization in which a ScrollView (or similar ViewGroups)
can ask to remove clipped subviews from the View hierarchy. This patch
implements this optimization for Nodes, but instead of adding and removing the
Views, it attaches and detaches Views instead.
Note that this patch does not handle overflow: visible. This is addressed in a
stacked patch on top of this patch (to simplify the review process).
Reviewed By: astreet
Differential Revision: D3235050
Summary:
Nodes crashed when setJSResponder was called on a virtual (non-View)
node, because a View could not be found using that react tag. The solution is
two fold - first, to figure out the View parent and pass that to
setJSResponder in addition to that of the virtual tag. Secondly, we weren't
mounting views that had animation properties (transform, for example) to
Views, which caused related code to fail.
Reviewed By: sriramramani
Differential Revision: D3301310
Summary:
Canvas.save by default saves both the matrix (for translations,
scaling, etc) and the clip (clipRect) - in most of our cases, we really only
care to save and restore the clip, not the matrix.
Reviewed By: sriramramani
Differential Revision: D3235698
Summary:
With nodes, it's possible for a touchable region to not be
explicitly mounted to a View. To work around this (and allow the region to
be the handler of the touch event), FlatViewGroup intercepts touch events
when the touch lies within any of the virtual NodeRegions.
This can sometimes be wrong - the canonical example is when touch starts
outside of a particular FlatViewGroup (so someone else, for example a
sibling) intercepts the touch event, and then the person moves over a
different FlatViewGroup, causing it to intercept the touch event when it
shouldn't. To fix this, we only allow intercepting touch events due to
NodeRegions on the down event.
Reviewed By: astreet
Differential Revision: D3160152
Summary:
By default, Nodes causes views to not be clipped, unless overflow is
explicitly set to hidden. Consequently, Nodes sets all the clipping bounds to
negative infinity, and does some extra work (saving the canvas layer,
clipping, etc) before drawing. This optimization skips the extra work when
it's not needed.
Reviewed By: sriramramani
Differential Revision: D3161268
Summary:
Initially, we used to mount nodes to Views anytime a node was
clicked. This was not useful, since we could still not handle touch when
a touch event was already dispatched. Later, a fix was pushed that
supported handling touch events for non-View NodeRegions. Part of the
intention was to remove this code, but it was forgotten.
Reviewed By: sriramramani
Differential Revision: D3160532
Summary: This allows users of the API to have greater control over handling RTL. One example is Components which needs this greater control to correctly handle RTL.
Differential Revision: D3120721
Summary:
UIImplementation has a few methods that in the end touch native Views, such as dispatchViewManagerCommand, addAnimation, sendAccessibilityEvent etc. There are 2 cases where it is possible to have those methods called on shadow nodes that don't have backing Views created:
- backing view is scheduled to be created but not commited yet (StateBuilder accumulates createView commands into a queue and flushes it in the very end)
- shadow node doesn't mount to a View so there is no backing View
Touching View in UI thread in these 2 cases will either lead to silent error (e.g. failure callback will execute), or a native crash.
This diff is overriding all UIImplementation methods that touch Views in UI thread and makes sure that backing View is created before we do so.
Reviewed By: ahmedre
Differential Revision: D3046392
Summary: In some rare cases, RCTText.measure can receive negative width which will crash with an assertion in android.text.Layout because it expects a positive value. This is a temporary fix to treat negative values as unconstrained width until the original bug is fixed.
Reviewed By: sriramramani
Differential Revision: D3038767
Summary:
Since we now have the correct fix in place, this patch is to revert
the hack we put in place to not be launch blocking.
Reviewed By: sriramramani
Differential Revision: D3019883
Summary: In FlatViewGroup, we flatten some react nodes into parent while mounting others into child Views. This is causing touch events being dispatched to wrong targets because child Views are \"stealing\" touch events from flattened Views. To fix the issue, implement ReactCompoundViewGroup to provide information about both virtual and non-virtual nodes.
Reviewed By: ahmedre
Differential Revision: D3018054
Summary: Before this patch, we only collected virtual nodes in NodeRegions, because NodeRegions are only needed to implement ReactCompoundView.reactTargetForTouch() which is only interested in virtual nodes. In the next patch, FlatViewGroup will implement ReactCompoundViewGroup interface which requires knowledge of both virtual and non-virtual children. As a step towards that, we need to include non-virtual nodes in NodeRegions. This patch is implementing that. By itself, it should have not cause any changes in application behavior: we add non-virtual nodes to NodeRegions and mark them as non-virtual, then skip all non-virtual nodes in reactTagForTouch().
Reviewed By: ahmedre
Differential Revision: D3018047
Summary: Before the patch, order of NodeRegions was inconsistent. Given parent A and 3 children B, C and D, we collect DrawCommands like this: A, B, C, D but NodeRegions were collected as B, C, D, A which neither matches draw order, nor a reverse of it. This patch changes it so that NodeRegions are collected in drawing order (A, B, C, D) and we iterate backwards to find correct touch target (in case they overlap).
Reviewed By: ahmedre
Differential Revision: D3018034
Summary:
This is a hack to fix the Groups dialog bug so that we can get some
real data in production from using Nodes. This should be replaced with a
better solution in the near future.
Differential Revision: D3016859
Summary: CSSNode.addChildAt() calls dirty() to invalidate the node and propagate dirty flag up to the root. However, ReactShadowNode overrides dirty() for virtual nodes so it does nothing. This results in bugs where an added text doesn't trigger a measure pass because RCTText is never dirtied. To fix the bug, override addChildAt() in RCTVirtualText and explicitly call notifyChanged(true) to make sure hosting RCTText is dirtied and re-measured/re-laid out.
Reviewed By: ahmedre
Differential Revision: D3016827
Summary:
This code reverts D3004541, since it fixes the symptom instead
of the root cause. Root cause fix is in D3011191.
Differential Revision: D3011291
Summary: @public Split dispatchViewUpdates into two methods, which enables subclasses to commit pending ui operations, even when no root node is present.
Differential Revision: D3011191
Summary:
getLayoutWidth is the same as right - left, and since we have right
and left, we can save a method call.
Reviewed By: sriramramani
Differential Revision: D3010697
Summary:
In D2980358, alignment was implemented for nodes. This unfortunately
introduced a bug, which is that when we have a BoringLayout that is set to
ALIGN_CENTER, it can't be seen. This is a result of the fact that the width
passed in to BoringLayout is Integer.MAX_VALUE. Since measure has already
been called at this point, we can just pass the layout's width as the width
of the BoringLayout.
Reviewed By: sriramramani
Differential Revision: D3004971
Summary:
Groups had a crash when running with React with Nodes when returning
from a search screen. This was due to the fact that the node representing a
ShimmerFrameLayout was being dropped, and then later we were trying to detach
it. Since the view was already dropped, we shouldn't try to detach it since
it's already dropped and removed from the view hierarchy.
Reviewed By: sriramramani
Differential Revision: D3004541
Summary:
We keep a list of FlatShadowNodes that mount to Views that we want to delete, and only flush it at the end of an update cycle. This results in a situation where a root view is being removed before children are removed, which results in a crash in NativeViewHierarchyManager because a View that we are trying to remove no longer exists. There are a few approaches to fix the issue:
a) make a check if a View exists before removing it. While works, it removes a bug protection when we erroneously trying to remove a View that no longer exists (such as this). I'd prefer to keep the check in place
b) flush the views-to-drop queue. This works, but does some extra work in UI thread (namely, removing Views that would be removed anyway)
c) trim the views-to-drop queue to remove any Views that will be removed anyway. This does a tiny bit of extra work in BG thread, but less work in UI thread.
This diff implements option c).
Reviewed By: ahmedre
Differential Revision: D2990105
Summary:
Code in FlatUIImplementation.manageChildren() incorrectly assumed that moveFrom is sorted and moveTo is not, which is actually reverse: moveFrom is not sorted, and moveTo is. This means that we need to sort moveFrom before we can traverse it, and that we no longer need to sort moveTo (we did it by moving nodes to mNodesToMove first and then sorting it).
The sorting algorithm used is borrowed from Android implementation of insertion sort used in DualPivotQuicksort.doSort() when number of elements < INSERTION_SORT_THRESHOLD(32) which is 99.999% the case in UIImplementation.manageChildren() (most of the time this array is either empty or only contains 1 element).
Another (very rare) bug this is fixing is that the code only worked for FlatShadowNodes, but not all shadow nodes are FlatShadowNodes (there are rare exceptions, such as ARTShape, ARTGroup etc). New code works with all types of shadow nodes.
Reviewed By: ahmedre
Differential Revision: D2975787
Summary: We are currently traversing entire tree every time there is any update to any of the nodes, which is hugely inefficient. Instead, we can early out for nodes that are known to contain no updates. This saves a lof of CPU. This optimization can be turned off, and an Exception will be thrown if there was an unexpected update.
Reviewed By: ahmedre
Differential Revision: D2975706
Summary:
Fix screenshot tests for React with nodes. It was broken due to
calling clipRect with bounds of [-∞, ∞], which, due to a bug in Canvas that
appeared in screenshot tests, caused the view not to draw. Since this is a
no-op anyway, this patch just doesn't call clipRect when we have infinite
bounds.
Differential Revision: D2975494
Summary: Right now invalidate always tell the root node that the tree is dirty, and next update will traverse the entire tree in search of changes. While this works correctly, it's not the most efficient implementation. It is more efficient to store dirty flag in every node, and skip entire subtrees if this node and all descendants are already up to date. This diff is a first step towards that optimization.
Reviewed By: ahmedre
Differential Revision: D2955197
Summary:
Examples that use defaultValue as part of their declaration (such as
those examples under UIExplorerApp's TextInput) were being ignored. This patch
supports having text set directly on RCTTextInput and handles it properly.
Note that this patch depends on D2962643 and D2964847, without which the
TextInput example will look wrong.
Differential Revision: D2968002
Summary:
There are 2 issues in removeChild() implementation.
a) There is a chance that a node that we are removing will be mounting to a View, but the create view request has not be created so there is no backing View for it yet. In that case, FlatNativeViewHierarchyManager.dropView() will throw an Exception failing to find the View. The fix is to only dropView() if one was (requested to be) created.
b) If the shadow node that we are removing doesn't mount to a View, but one or more of its children do, those Views will leak because noone is removing them. The fix is to recursively call removeChild() until we find a node that mounts to a View.
Reviewed By: ahmedre
Differential Revision: D2974938
Summary:
OnLayoutEvent was incorrectly dispatching x/y coordinates relative to native View instead of relative to parent. This was causing issues in many cases, such as when Node was mounting to a View, in which case x/y would always be 0. This diff is fixing it.
This diff also caches last OnLayoutEvent to avoid dispatching the event on every layout even when layout didn't chance.
Reviewed By: ahmedre
Differential Revision: D2955087
Summary:
Setting the color for a TextInput with nodes was broken. The text color was
not being applied due to an optimization that prevented setting spans if
begin and end were the same (which is the case for an empty TextInput).
This patch depends on D2962643.
Differential Revision: D2962394
Summary:
The spannable flags for RCTVirtualText were always being set to
INCLUSIVE_EXCLUSIVE - this is different than the behavior that is found in
ReactTextShadowNode, which sets EXCLUSIVE_INCLUSIVE as the default flag
unless the text is at the beginning.
This is needed to fix various problems with TextInput, including the handling
of empty spans (see also D2962394 which depends on this patch), and making the
behavior consistent when styled children of a TextInput are changed.
Differential Revision: D2962643
Summary:
Implement measure for RCTTextInput. This is (almost) the same as
the one implemented for ReactTextInputShadowNode.
Differential Revision: D2964847
Summary:
There are 2 reasons why someone would call StateBuilder.ensureBackingViewIsCreated():
1) to make sure a View is created, because we are going to use it NOW
2) make sure react styles are applied to View, which doesn't really need the View to be created immediately
This diff is splitting the method into 2, without changing behavior. Difference between the methods' signatures is coming from the fact that 1) never takes styles and 2) possibly takes styles.
This is a pure refactoring diff and should have no change in functionality or behavior.
Reviewed By: ahmedre
Differential Revision: D2916697
Summary:
There is a rare case in React Native when a hierarchy is created and then *immediately* discarded all in one transaction. This is causing a View leak, because a View is created and then never attached to a hierarchy because it shadow node got detached. Since we never attached the View to the hierarchy, we cannot do a detach either and the View is kept in NativeViewHierarchyManager forever, causing a View leak. To fix the issue, don't create a backing View whenever a shadow node mounts to a View. Instead, put the create request into a queue and ONLY execute it if a shadow node is still attached to a hierarchy. This is fixing the View leak because will not create a View unless it's going to be attached somewhere.
The same logic applies to View style update (we don't want to update a View that is going to be detached from a hierarchy).
Reviewed By: ahmedre
Differential Revision: D2916826
Summary:
There are 2 reasons why someone would call StateBuilder.ensureBackingViewIsCreated():
1) to make sure a View is created, because we are going to use it NOW
2) make sure react styles are applied to View, which doesn't really need the View to be created immediately
This diff is splitting the method into 2, without changing behavior. Difference between the methods' signatures is coming from the fact that 1) never takes styles and 2) possibly takes styles.
This is a pure refactoring diff and should have no change in functionality or behavior.
Reviewed By: ahmedre
Differential Revision: D2916697
Summary:
When layout happens, left/top/right/bottom coordinates (and thus clip-*, too) are sometimes not at a pixel boundary for 2 reasons:
a) width/height in Flexbox are floats, and can contain any value, not just a whole pixel. E.g. style={{width: 3.1415926}} is perfectly valid
b) floating point arithmetics sometimes leads to values barely outside of pixel boundaries (width 18.0f can become 18.000001f when a sibling/child size changes).
a) is \"breaking\" screenshot tests, which slightly but differ from reference implementation
b) causes extra View updates/redraws for no reason
This patch is rounding DrawCommand bounds to a whole pixel to avoid these 2 issues.
Reviewed By: ahmedre
Differential Revision: D2934401
Summary:
Public
We wanted to do this before, but couldn't because touch events had timestamps
set by the system (and matched System.currentTimeMillis), but now we set those
timestamps! The idea behind this change is that System.currentTimeMillis is
unreliable, but nanoTime isn't, and it also guarantees that we will never have two events with the same
timestamp.
We're still seeing crashes with touch events not ending correctly in JS, this
might be the cause of that.
Reviewed By: foghina
Differential Revision: D2953917
Summary:
Many of StateBuilder methods take a lot of arguments, which makes it hard to read. This patch is doing a bit of cleanup by removing tag from the method signatures, because tag can be easily obtained from the node itself.
This is a pure refactoring diff and should have no functional changes.
Reviewed By: ahmedre
Differential Revision: D2915815
Summary: Both DrawView and AbstractDrawCommand have clipping logic, this diff is moving out logic into a common base class. This also reverts the screenshot tests fix, which was causing issues with overflow: visible.
Reviewed By: ahmedre
Differential Revision: D2933780
Summary:
The current AndroidView stipulates that the backing shadow node can't
be a FlatShadowNode. In some cases, however, we want to apply some of the same
logic (ex not adding NodeRegions, etc) to other ViewManagers that have a
FlatShadowNode backing (and that don't necessarily create a FlatViewGroup).
This commit renames AndroidView to NativeViewWrapper, and re-introduces
AndroidView as an interface, so that logic for padding, NodeRegions, etc can
be shared.
Differential Revision: D2942387
Summary: Not every CSSNode in a hierarchy is a FlatShadowNode, some virtual nodes can be ReactShadowNodes for compatibility with ART nodes. This diff fixes StateBuilder unconditionally casting a node to FlatShadowNode, which is causing a crash in Groups.
Reviewed By: nickholub
Differential Revision: D2941031
Summary: These methods (mostly getters/setters) are not meant to be overriden, so let's make them final. Hopefully, they will inline better, too.
Reviewed By: ahmedre
Differential Revision: D2933869
Summary:
@public Relax the constraint on ReactTextInputManager.
The TextInput Advanced screen looked different with and without
nodes, namely child Text items were not being rendered on the Nodes version.
This patch fixes that.
Differential Revision: D2930800
Summary: FlatUIImplementation.handleCreateView was missing an Override annotation. This diff fixes it.
Reviewed By: sriramramani
Differential Revision: D2919596
Summary: Currently, we wrap all unknown shadow nodes with AndroidView. This works great, except when the shadow node is virtual, i.e. it *doesn't* mount to a View. In this case, we just need to keep it in the hierarchy as is. Fixes ARTSurfaceView not working correctly in groups.
Reviewed By: ahmedre
Differential Revision: D2933325
Summary: Software Canvas uses ints to represents boundaries (because it is backed by Bitmap that has scalar size), whereas Hardware Canvas uses floats to represent boundaries. This results in a bug in Software Canvas where clipping with floats close to or outside of int range results in int overflows and incorrect clipping. To fix the issue, compute the clip boundaries manually instead of using Canvas.clipRect() method (that contains the bug).
Reviewed By: sriramramani
Differential Revision: D2919509
Summary: There is a bug in border drawing code that will not draw the border if the element has a background color. This diff fixes it.
Reviewed By: sriramramani
Differential Revision: D2919549
Summary: ImageRequestHelper was not handling data: scheme correctly, which resulted in images failing to load. This diff is fixing it by considering \"data:\" as a Uri resource, and piping it appropriately.
Reviewed By: sriramramani
Differential Revision: D2919403
Summary: Previously, RCTImageView/DrawCommand would always clip for specific scale types (such as CENTER_CROP), but this is incorrect if we want to allow overflow: visible on RCTImageView (which iOS allows). This patch tweaks the clipping condition from paddingLeft/paddingTop < 0 (i.e. image wants to draw outside of its bounds) to actual image rect being outside of clip rect. Previously, that would mean exact same thing, but now that clip rect is aware of overflow: visible, it matters, allowing RCTImage to not clip when we want it to draw outside of the boundaries.
Reviewed By: ahmedre
Differential Revision: D2873350
Summary:
Previously, every node would be clipped by its own boundaries. This made sense, because
a) RCTView cannot draw outside of its bounds, so clipping is harmless
b) all other node types were not allowed to control overflow behavior, and for them overflow was implicitly set to hidden, which means it should clip.
This diff changes the logic so that overflow can be set on any node (image, text, view etc).
The change has an important implication that make the diff a little trickier that it could be: AbstractDrawCommand clipping logic needs to be relaxed. Previously, clip bounds were *always* intersected with node bounds, which means that clip rect was always within node rect. This is no longer true, and thus shouldClip() logic needs to be modified.
Because of the above, RCTText no longer needs to artificially clip bounds against layout width/height.
Note: RCTImage is still not working with overflow: visible correctly (it still always clips). This is fixed in a follow up diff.
Reviewed By: ahmedre
Differential Revision: D2867849
Summary:
There is an assert in FlatViewGroup.reactTagForTouch() that says that TouchTargetHelper should not allow returning getId() when pointer events are BOX_NONE. This is not entirely accurate.
This acturally method is invoked in 2 different contexts. Main context is to find a touch target, and in that context the method indeed should never return getId() if pointer events are BOX_NONE. There is however a TouchTargetHelper which actually expects that reactTagForTouch() *may* return getId(), in which case it will perform logic to not all this method be invoked again from main context.
In other words, this assert needs to be removed because it is entirely possible to return getId() when pointer events are BOX_NONE. Ideally, these would be 2 different methods, but ReactCompoundView interface only defines a single reactTagForTouch() method.
Reviewed By: ahmedre
Differential Revision: D2873931
Summary: When padding is applied to RCTText, the text it renders should be offset by that padding. This is what iOS is doing.
Reviewed By: sriramramani
Differential Revision: D2872039
Summary: A simple refactoring diff that should make code a little easier to read. No functional changes.
Reviewed By: ahmedre
Differential Revision: D2867718
Summary: Exact same class is needed for Groups, and probably for other apps that want to work with Nodes, too, so let's extract it into a helper class to simplify coding.
Reviewed By: sriramramani
Differential Revision: D2873943
Summary:
Nodes create a FlatViewGroup for a root node element, and NativeViewHierarchyManager assigns it an id that matches root node it. Now this top-level FlatViewGroup is added to a ReactRootView. In non-nodes, this ReactRootView has an id that matches root View id, and when this ReactRootView gets detached from Window, it sends an event to NativeViewHierarchyManager that says that all the children of this View must be removed recursively. This works great in non-Nodes, but fails in Nodes because ReactRootView has no id (-1) and NativeViewHierarchyManager fails to find a corresponding ReactShadowNode, throwing an Exception. To fix the issue and make it work again we need to assign this View id of the top level shadow node. This creates a minor issue, where 2 Views (ReactRootView and its only child) share the same id, but that is not a problem because we don't enforce uniqueness of the ids, and don't rely on getViewById().
This was originally implemented, but then I removed it because I thought it wasn't truly needed. Turns out, it is needed.
Reviewed By: sriramramani
Differential Revision: D2873995
Summary: This diff implements ImageLoadEvents (ON_LOAD, ON_LOAD_END and ON_LOAD_START) in RCTImageView. ON_ERROR and ON_PROGRESS are easy to implement, too, but these 2 are supposed to carry extra information (error message and progress) but ImageLoadEvent doesn't support a payload yet (I'll add them in a separate patch).
Reviewed By: ahmedre
Differential Revision: D2824772
Summary: React provides properties to support shadows for Text, this diff implements it for Nodes.
Reviewed By: ahmedre
Differential Revision: D2817212
Summary: Minor optimization/cleanup: instead of creating an instance of FontStylingSpan for every RCTVirtualText, start with a global immutable instance of use copy-on-write when any of the properties change.
Reviewed By: ahmedre
Differential Revision: D2817156
Summary: Wrong order or top/right -> right/top means that clipping can be incorrect. I didn't notice it because visual result in many cases would still be the same (especially with overflow: visible being default and clipping being mostly disabled). Also only background was clipped incorrectly, and most texts don't have a background.
Reviewed By: ahmedre
Differential Revision: D2816432
Summary: Small optimization that prevents empty NodeRegions from being collected. Those will never receive touch events anyway, so why bother allocating memory and iterating over them?
Reviewed By: ahmedre
Differential Revision: D2816355
Summary: RCTText may modify its DrawCommand on collectState(), which updateNodeRegion() relies on. This means that order of the two is important: call collectState() first, followed by updateNodeRegion(). This fixes the bug where am RCTText may sometimes not respond to clicks (because its NodeRegion would be empty).
Reviewed By: ahmedre
Differential Revision: D2816295
Summary: NodeRegions are touch regions within hosting View, and while in most cases they are the same as View boundaries, there is one case where it's not true: TextNodeRegion. When mounted to a View, TextNodeRegion will have a bounds of (0,0,width,height) which is clearly different from (left,top,right,bottom). Initially I assumed they would always be the same so we could use information stored in NodeRegion (should probably be called TouchRegion) to update node's View boundaries, but it breaks RCTTextView when it mount to a View (because it would either contain incorrect bounds, or View will be laid out incorrectly). Right now touch is not working on RCTView that mounts to a View. To fix the issue, separate the 2 concepts.
Reviewed By: ahmedre
Differential Revision: D2816268
Aaron Chiu
Pieter De Baets
Gerald Monaco
Emil Sjolander
Lukas Piatkowski
Janic Duplessis
Philipp von Weitershausen
Rene Weber
Ashok Menon
Ahmed El-Helw
Kevin Gozali
sunnylqm
Jakub Zika
Satyajit Sahoo
Andy Street
Maël Nison
Martin Konicek
Vince Yuan
Michael Lee
Charles Dick
Luke Miles
Lukas Woehrl
Konstantin Raev
Caleb Gomer
Theo Yaung
Sriram Ramasubramanian
Jonathan Lawlor
Dmitry Petukhov
Felix Oghina
Andrei Coman
Andrew Y. Chen
Seth Kirby
Mengjue Wang
Dave Miller
Adam Comella
Denis Koroskin
