From 8014147013cfa3f907a7b953a00bc8e0e2b00efe Mon Sep 17 00:00:00 2001 From: Denis Koroskin Date: Mon, 7 Mar 2016 20:06:30 -0800 Subject: [PATCH] Fix order of NodeRegions 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 --- .../facebook/react/flat/FlatViewGroup.java | 3 ++- .../com/facebook/react/flat/StateBuilder.java | 21 ++++++++++--------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java index 040cec66d..5e2a2385f 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/FlatViewGroup.java @@ -412,7 +412,8 @@ import com.facebook.react.views.image.ImageLoadEvent; } private NodeRegion nodeRegionWithinBounds(float touchX, float touchY) { - for (NodeRegion nodeRegion : mNodeRegions) { + for (int i = mNodeRegions.length - 1; i >= 0; --i) { + NodeRegion nodeRegion = mNodeRegions[i]; if (nodeRegion.withinBounds(touchX, touchY)) { return nodeRegion; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java index 7df5db8b4..e5be9c10b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/StateBuilder.java @@ -285,6 +285,15 @@ import com.facebook.react.uimanager.events.EventDispatcher; clipBottom = Float.POSITIVE_INFINITY; } + if (!isAndroidView && node.isVirtualAnchor()) { + // If RCTText is mounted to View, virtual children will not receive any touch events + // because they don't get added to nodeRegions, so nodeRegions will be empty and + // FlatViewGroup.reactTagForTouch() will always return RCTText's id. To fix the issue, + // manually add nodeRegion so it will have exactly one NodeRegion, and virtual nodes will + // be able to receive touch events. + addNodeRegion(node, left, top, right, bottom); + } + boolean descendantUpdated = collectStateRecursively( node, left, @@ -298,15 +307,6 @@ import com.facebook.react.uimanager.events.EventDispatcher; isAndroidView, needsCustomLayoutForChildren); - if (!isAndroidView && node.isVirtualAnchor()) { - // If RCTText is mounted to View, virtual children will not receive any touch events - // because they don't get added to nodeRegions, so nodeRegions will be empty and - // FlatViewGroup.reactTagForTouch() will always return RCTText's id. To fix the issue, - // manually add nodeRegion so it will have exactly one NodeRegion, and virtual nodes will - // be able to receive touch events. - addNodeRegion(node, left, top, right, bottom); - } - boolean shouldUpdateMountState = false; final DrawCommand[] drawCommands = mDrawCommands.finish(); if (drawCommands != null) { @@ -549,6 +549,8 @@ import com.facebook.react.uimanager.events.EventDispatcher; updateViewBounds(node, left, top, right, bottom); } } else { + addNodeRegion(node, left, top, right, bottom); + updated = collectStateRecursively( node, left, @@ -561,7 +563,6 @@ import com.facebook.react.uimanager.events.EventDispatcher; parentClipBottom, false, false); - addNodeRegion(node, left, top, right, bottom); } return updated;