Remove 'box-only' as a layoutOnlyProp #15529
Conversation
Currently, having the property `pointerEvents={'box-only'}` marks a view as layoutOnly. For optimization reasons, layoutOnly nodes are [not added to the node hierarchy](https://github.com/facebook/react-native/blob/b103903ec869bc48dfcaf001dc926957d0b5200a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyOptimizer.java#L99) The problem is that should this box ever need to be [transitioned to not-layout-only](https://github.com/facebook/react-native/blob/b103903ec869bc48dfcaf001dc926957d0b5200a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyOptimizer.java#L394) (for example, because you add the a CSS transform property), it must be added to the hierarchy. To add it to the hierarchy the [React Styles Diff Map](https://github.com/facebook/react-native/blob/b103903ec869bc48dfcaf001dc926957d0b5200a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyOptimizer.java#L396) is passed along with the backing shadowNode to create the node. The problem is that at no point were the original `pointerEvents` and so the new node will be created with `pointerEvents = 'auto'`.
A more correct fix to this problem might be to have shadowNodes carry around their pointerEvents, although this is likely a greater design decision which is why am I proposing the quick fix now.
Will also resolve: react-native-modal/react-native-modal#11
facebook-github-bot
added
CLA Signed
@facebook-github-bot label Needs more information Generated by 🚫 dangerJS |
|
Yes, checkout the issue I pointed to on react-native-modal that describes
the issue more in depth.
…On Aug 19, 2017 6:00 PM, "Valentin Shergin" ***@***.***> wrote:
@tomasreimers <https://github.com/tomasreimers> This in fact affects only
Android, right?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#15529 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABIkPVG0rudBGl2-QYux6P1sc5hZKe1Eks5sZ1sNgaJpZM4O57nR>
.
|
|
+1 I can confirm this caused some headaches in the #11 |
|
I even don't understand how |
|
I suspect pointerEvents are related because one of the reasons we'd create a real view (instead of only a shadow view) is so it can receive touch events. It seems to me there are two bugs in the logic. We should check for some properties that always require a real view (ex: transform) and make sure they force layout-only to be false. We also should have the shadow views carry more information as mentioned in the first post above so that we can turn a layout-only view into a real view correctly. |
|
Maybe just ship this as is and add a todo comment + issue to improve. |
facebook-github-bot
added
the
Import Started
|
@tomasreimers has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
Currently, having the property
pointerEvents={'box-only'}marks a view as layoutOnly. For optimization reasons, layoutOnly nodes are not added to the node hierarchy The problem is that should this box ever need to be transitioned to not-layout-only (for example, because you add the a CSS transform property), it must be added to the hierarchy. To add it to the hierarchy the React Styles Diff Map is passed along with the backing shadowNode to create the node. The problem is that at no point were the originalpointerEventsand so the new node will be created withpointerEvents = 'auto'.A more correct fix to this problem might be to have shadowNodes carry around their pointerEvents, although this is likely a greater design decision which is why am I proposing the quick fix now.
Will also resolve: react-native-modal/react-native-modal#11