Commit Graph

30 Commits

Author SHA1 Message Date
Samuel Susla fdd133e214 Prevent getRelativeLayoutMetrics to measure across ShadowNode which is root
Summary:
Changelog: [Internal]

# Analysis
Measure returns following values for `frame.y` when tapping item in bottom sheet.

Fabric 412.33331298828125.
Paper 49.

In Paper, the frame.y returned is the position of tapped item in bottom sheet relative to the bottom sheet itself, which is correct.

This can happen in both BottomSheet and Modal.

# Why it happens?
In [UIManager.getRelativeLayoutMetrics](https://our.intern.facebook.com/intern/diffusion/FBS/browse/master/xplat/js/react-native-github/ReactCommon/fabric/uimanager/UIManager.cpp?commit=a372cf516ba1245ad9462e68376ee759c118a884&lines=172-181) if `ancestorShadowNode` is nullptr we populate `ancestorShadowNode` with `rootShadowNode` for the surface.
Problem is that BottomSheet that is presented, is not a separate surface. This means that we climb up the shadow node hierarchy all the way to root shadow node and keep adding offsets. Even though we should stop when we hit shadow node representing BottomSheet.

# How could be this fixed?

I think we should add a new shadow node trait that would mark node as root node. As we are traversing the shadow node tree upwards and adding offset of that node, once we hit a node that is "anchor",  we would immediately stop the traversal.
This solution is inspired by Paper where the node representing BottomSheet has a special flag which marks it as "root" node.

accepttoship

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D19640454

fbshipit-source-id: bde623b1f41a9745a41f0aada7221bf924fad453
2020-01-30 15:30:39 -08:00
Samuel Susla 01805636b9 Move ShadowNode::getAncestors to ShadowNodeFamily
Summary:
Changelog: [internal]

1. Moves `ShadowNode::getAncestors` to `ShadowNodeFamily`.
2. Exposes shadowNode's family through `ShadowNode::getFamily()`.

# Why?
This is a first step in order to merge `StateCoordinator` into `ShadowNodeFamily` and use it as target for state updates.

Reviewed By: shergin

Differential Revision: D19465188

fbshipit-source-id: b5a3625aa21c040301259de02beedbf97e11f20e
2020-01-28 09:32:52 -08:00
Samuel Susla 510dbc5ec1 Fix measureLayout when relative node is the same as this node
Summary:
# Changes
1. Fixes a bug when calling `LayoutableShadowNode::getRelativeLayoutMetrics` with `this` being the same node as ancestor.
2. Refactors logic that increments `layoutMetric.frame.origin` by iterating through ancestors.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D19468690

fbshipit-source-id: 5b9e187adc26a206da035e4387bb5f528aabdbb2
2020-01-22 05:12:10 -08:00
Samuel Susla b3af384a13 Fix wrong layoutMetrics being returned by measure call
Summary:
# Logs
```
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 0   tag: 1432 debugValue: r1
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r2
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r2
[tid:com.facebook.react.JavaScript][UIManager.cpp:197]            width: 0   tag: 1432 debugValue: r1/sealed <----------------- FIRST TAP
[tid:com.facebook.react.JavaScript][UIManager.cpp:197]            width: 0   tag: 1432 debugValue: r1/sealed <----------------- FIRST TAP
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r4
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r5
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r6
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r7
[tid:com.facebook.react.JavaScript][ConcreteViewShadowNode.h:119] width: 375 tag: 1432 debugValue: r8
[tid:com.facebook.react.JavaScript][UIManager.cpp:197]            width: 375 tag: 1432 debugValue: r7/sealed <----------------- SECOND TAP
[tid:com.facebook.react.JavaScript][UIManager.cpp:197]            width: 375 tag: 1432 debugValue: r7/sealed <----------------- SECOND TAP
```
# What’s happening here?

Opening a *BottomSheet* and tapping the first item. As you can see before the item is tapped, it has *width* set to 375 in revision *r2*.
When the tap happens, JavaScript is requesting an old revision of ShadowNode, which still has width 0.

# My assumption.

1. Native creates node with *width* 0 and returns handle to *JS*.
2. Native *clones* the node, sets its *width* to 375, doesn’t tell *JS* about it. This update is due to state change.
3. *JS* tries to get the size of the node, but asking for first revision of the component it receives width 0.

# Other observations

1. Manually setting width to 375 in UIManager::getRelativeLayoutMetrics fixes the problem.
2. This happens only on device, I wasn’t able to reproduce this on simulator.

# Fix

Find the newest revision of ShadowNodeFamily, and return its layoutMetrics.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D19433873

fbshipit-source-id: 4558cf6e704051e9b3968e83821d8d25b3dadcda
2020-01-20 01:31:22 -08:00
Samuel Susla 5b156f83fa Fix origin calculation in getRelativeLayoutMetrics
Summary:
# Problem
The issue is in implementation of `LayoutableShadowNode::getRelativeLayoutMetrics`.
If you have view tree (A has a child B), and you call `B.getRelativeLayoutMetrics(A)`, the expected result is B's origin within A. The implementation wasn't reflecting that, it was reflecting B's origin within A + A's origin within its parent.

# Fix
When iterating over ancestors of ShadowNode, the last ancestor should be skipped.
AncestorList is a list that starts with provided ancestor and ends with parent of `this`. To skip provided ancestor we iterate to `rend() - 1`.

# Why does it work in some cases?
This function is triggered from `UIManager.getRelativeLayoutMetrics` without `ancestorShadowNode` provided, we find the RootShadowNode, which has origin `{0, 0}`.

Changelog: [Internal]

Reviewed By: shergin

Differential Revision: D19447900

fbshipit-source-id: 4a9606dc1fab3fecfb85d337b014188d80e5b355
2020-01-20 01:31:22 -08:00
David Vacca 5e7a03f7b6 Implement findNodeAtPoint method
Summary:
This diff implements the findNodeAtPoint method to return the ShadowNode that is positioned into a Point of the screen.

What's not supported:
- Scroll position
- Transform
- return layoutable nodes that are contained inside a non-layoutable node

Changelog: [internal]

Reviewed By: shergin

Differential Revision: D19190285

fbshipit-source-id: fdc0358dc21312e9950a4eb16c36020e9e43e33f
2019-12-23 17:09:44 -08:00
Valentin Shergin c89ead313c Fabric: Proper implementation of ScrollView::contentOffset-aware measure functions (iOS only for now)
Summary:
This diff re-enables propagation of ScrollView's content-offset value down to the ShadowTree layer and enables measure fucntions opt-in incorporating this info to result.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: zackargyle

Differential Revision: D18981891

fbshipit-source-id: a6c0f4e690703b0ee07d45efab161750cfcc4b60
2019-12-18 10:48:40 -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
Valentin Shergin fb85e90487 Fabric: Using LayoutableShadowNode::getTransform in measuring
Summary: After this change, all measuring operations based on `LayoutableShadowNode::getRelativeLayoutMetrics` will take into account the transformation matrices provided by shadow nodes. This will allow implementing scroll-offset-aware and custom-transform-aware measuring.

Reviewed By: JoshuaGross, mdvacca

Differential Revision: D15219259

fbshipit-source-id: 1d7cd8c0ee4406f4dd0002cd442dc0679391922b
2019-05-06 17:06:40 -07:00
Valentin Shergin 0e35406e4e Fabric: LayoutableShadowNode::getTransform
Summary:
`LayoutableShadowNode::getTransform` returns a transform object that represents transformations that will/should
be applied on top of regular layout metrics by mounting layer.

Reviewed By: mdvacca

Differential Revision: D15219262

fbshipit-source-id: e7aeb85b5f7e2fce3f8faf9dfcaee5dae3217d36
2019-05-06 17:06:40 -07:00
Valentin Shergin f24fd6e9ee Fabric: Using new shiny ShadowNode::getAncestors in LayoutableShadowNode
Summary: Same algorithm using a new API.

Reviewed By: JoshuaGross

Differential Revision: D14423509

fbshipit-source-id: c40f61fdd001f68785512c304941f523585d641c
2019-03-18 23:56:13 -07:00
Valentin Shergin 55eaf30740 Fabric: Checking for HasNewLayout before applying layout metrics to ShadowNode
Summary:
If the `HasNewLayout` flag is `false`, we should not copy the data from YGStyle/YGLayout to ShadowNode because the node can be already sealed and because it's unnecessary. Previously we workaround this case with a special check in `setLayoutMetrics` method, but that can be unreliable because of some side-effects related to pixel rounding and comparing floats.
The new approach is much more robust and explicit.

Reviewed By: JoshuaGross

Differential Revision: D14496939

fbshipit-source-id: deddb14d2206c5bd3f22154d0ea682e3c5888901
2019-03-18 19:21:26 -07:00
Valentin Shergin be6fa25d16 Fabric: Removing default implementation of some methods of LayoutableShadowNode
Summary: These default implementations are never being used. Removing them allowing to ensure that flags inside YGNode have same values as flags in YogaLayoutableShadowNode. That also saves a couple of bytes in size of ShadowNode.

Reviewed By: JoshuaGross

Differential Revision: D14496938

fbshipit-source-id: c43f9c8a2eec054f728ff54a6573668eccda55fb
2019-03-18 19:21:26 -07:00
Valentin Shergin 9eec2c33e6 Fabric: LayoutableShadowNode::getRelativeLayoutMetrics()
Summary:
The generic method that returns relative (to some specified node) layout metrics.
We need this as a building block to implement `UIManager.measure*()` methods family.

Reviewed By: mdvacca

Differential Revision: D12932549

fbshipit-source-id: 79064551d32b0cd40e72dc87d0ed194e3779ba29
2018-11-21 17:16:48 -08:00
Valentin Shergin 94d49e544d Fabric: Codemod: All <fabric/... includes were renamed to <react/...
Summary: We are moving to more stable APIs removing all mentiones of the effort name from the codebase.

Reviewed By: mdvacca

Differential Revision: D12912894

fbshipit-source-id: 4a0c6b9e7454b8b14e62d419e9e9311dc0c56e7a
2018-11-10 14:22:15 -08:00
Valentin Shergin 8f51243957 Fabric: Enabling clang-format for the rest of Fabric
Summary: This is the second and the final part of adopting clang-format.

Reviewed By: mdvacca

Differential Revision: D10229624

fbshipit-source-id: d97670b716800ea2488b84bd0aacaf54d8bd2e31
2018-10-09 16:31:48 -07:00
Valentin Shergin e3b61f53bd Fabric: Debug Pretty-printing is now debug only feature
Summary: That should save us some app size kilobytes.

Reviewed By: mdvacca

Differential Revision: D10081499

fbshipit-source-id: 2b950768c609b412f9be332c22b6b1e96657e5ea
2018-09-28 10:32:06 -07:00
Héctor Ramos 1151c096da Update copyright headers to yearless format
Summary: This change drops the year from the copyright headers and the LICENSE file.

Reviewed By: yungsters

Differential Revision: D9727774

fbshipit-source-id: df4fc1e4390733fe774b1a160dd41b4a3d83302a
2018-09-11 15:33:07 -07:00
Valentin Shergin 9570d7d490 Fabric: Unifying usage of autos
Summary:
I was watching a classic magnificent talk about modern C++ by Herb Sutter and I was totally sold on double down on using `auto` in our codebase. Surprisingly, 95% of the code base already follows Herb's guidence; I just changed the last 5% to make it consistent.
All those changes must work *exactly* like it was before.
The talk: https://youtu.be/xnqTKD8uD64?t=28m25s

Reviewed By: mdvacca

Differential Revision: D9753301

fbshipit-source-id: 9629aa485a5d6e51806cc96306c297284d4f90b8
2018-09-10 16:50:02 -07:00
Valentin Shergin 1068da2ec7 Introducing LayoutableShadowNode::isLayoutOnly and (theoretical) view-flattening
Summary:
@public
Voalá, this small change actually implements view flattening. Obviously, it does not work right now because there are no `ShadowNode` classes which implement `isLayoutOnly`.
Surprisingly, correct implementing of `isLayoutOnly` is quite tricky, we will work on this in coming diffs.

Reviewed By: mdvacca

Differential Revision: D9403565

fbshipit-source-id: 1f16f912cb5c6841405a1fc3cf36aec28698c11f
2018-09-03 23:04:20 -07:00
Valentin Shergin e906d4cdc9 Simplifying child nodes management in YogaLayoutableShadowNode
Summary:
@public

This diff consists of many interdependent changes which support one simple idea: YogaLayoutableShadowNode is now using YGNode children to iterate on them (it previously relied on `ShadowNode::getChildren()`). All other changes are just an unavoidable consequence of that. Hence we don't need to filter child nodes every single time when we do layout anymore! The logic around `clone callback` is also drastically simpler now.
The new approach also implies that `LayoutableShadowNode` and `YogaLayoutableShadowNode` don't use `shared_ptr`s to refer to ShadowNode objects because new relationship does not imply ownership. No more `SharedShadowNode` objects in those two classes.

Reviewed By: mdvacca

Differential Revision: D8796159

fbshipit-source-id: 6f52f92d1826f3eb13b2f8a132c3ea77de155d82
2018-07-17 22:53:56 -07:00
Valentin Shergin 9f85873c9f Fabric: New props treatment in graphics module
Summary:
Same as previous one.
Adopting template-generated `convertRawProp` and `debugStringConvertibleItem` functions in `graphics` module.
Note, to do so we have to change signatures of some conversions functions to make them more overloading-friendly.

Reviewed By: fkgozali

Differential Revision: D7958252

fbshipit-source-id: 0f33a2e6aad60befacee31486acdb9b6114d3e07
2018-05-14 16:03:49 -07:00
Valentin Shergin 98b4747041 Fabric/View: Debug printing logic in YogaLayoutableShadowNode was moved to superclass
Summary:
It's more useful and consistent now because:
 - We print compound layout metrics from the correct storage (not from Yoga nodes);
 - It works fro any kind of layout now (but we still have just one);
 - It's much clear and straight-forward.

Reviewed By: fkgozali

Differential Revision: D7607422

fbshipit-source-id: 4c3cd2848e785a7f77c7f591e376d00c7c09ade9
2018-04-26 18:03:07 -07:00
Valentin Shergin b13d5beb11 Fabric: LayoutableShadowNodeList::getChildren renamed to getLayoutableChildNodes
Summary:
The previous name conflicts with the method with same (but with different semantic) name in `ShadowNode` class.
That was bad idea to use same name especially because the different semantic.

Reviewed By: fkgozali

Differential Revision: D7554549

fbshipit-source-id: 0bccbaacd0812f8a26592b2008f15ddb5bc34ebc
2018-04-10 17:15:08 -07:00
Valentin Shergin 711abeda79 Fabric: Using enum class for some graphics types
Summary:
`enum class` types do not provide default conversion to integers and reguire use typename before value names.
This must prevent bugs like the previous one where we used `Undefined` as a number value.

Reviewed By: fkgozali

Differential Revision: D7554548

fbshipit-source-id: b19379aae48c9aebe03043e08cf3acc712da3cb8
2018-04-10 17:15:08 -07:00
Valentin Shergin db92b16e04 Fabric: Proper invocation of ensureunsealed in LayoutableShadowNode::layout
Summary: Quite obvious. We have to check `sealable` flag of the child (not a parent) before mutating it.

Reviewed By: mdvacca

Differential Revision: D7526407

fbshipit-source-id: ce8e0d6446ff7eb23baee9c92f246d5e198fe377
2018-04-10 17:15:08 -07:00
Valentin Shergin 7e84cadc9c Fabric: Refined conception and usage of Sealable
Summary: Slightly new approach: Some non-const methods might not always mutate objects, so sometimes we should call `ensureUnsealed()` only inside conditional branches where we actually mutate an instance.

Reviewed By: fkgozali

Differential Revision: D7467793

fbshipit-source-id: 1b9f229cf6816e54e0df36699a571fdb612d3c3c
2018-04-10 12:59:50 -07:00
Valentin Shergin ee0cc6bbe7 Fabric: Implementing cloneAndReplaceChild() for ViewShadowNode
Summary:
First, LayoutableShadowNode::cloneAndReplaceChild() is now pure virtual. The default implementation was useless and confusing.
Second, cloneAndReplaceChild() is now fully implemented for ViewShadowNode, so fancy Yoga Concurrent layout *should* work now.

Reviewed By: mdvacca

Differential Revision: D7376352

fbshipit-source-id: 1199a37e64535c8592a2a5480e60139f61d02006
2018-03-25 22:48:18 -07:00
Valentin Shergin a5a34565e0 Fabric: Proper return value of LayoutableShadowNode::setLayoutMetrics()
Summary: `LayoutableShadowNode::setLayoutMetrics()` must return `false` is nothing was changes.

Reviewed By: fkgozali

Differential Revision: D7330334

fbshipit-source-id: 700d50b0047919fa2b919acfa72825f100cd496f
2018-03-19 17:08:31 -07:00
Valentin Shergin 811d5bfc73 Introducing LayoutableShadowNode
Summary: LayoutableShadowNode defines a unified interface (and set of primitives) essential for laying out shadow nodes.

Reviewed By: fkgozali

Differential Revision: D7230668

fbshipit-source-id: d8c1772d4c3bd1f87c41f7240a39aecebf3696ae
2018-03-18 19:17:39 -07:00