From c62840c7a8c7e8780476c15558edd6c00a10f942 Mon Sep 17 00:00:00 2001 From: Ahmed El-Helw Date: Mon, 16 May 2016 12:35:08 -0700 Subject: [PATCH] RCTVirtualText wasn't clickable in certain cases 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 --- .../java/com/facebook/react/flat/RCTText.java | 17 ++++++++-- .../facebook/react/flat/TextNodeRegion.java | 34 ++++++++++++------- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java index 6564f284a..05e125670 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/RCTText.java @@ -179,6 +179,7 @@ import com.facebook.react.uimanager.annotations.ReactProp; return; } + boolean updateNodeRegion = false; if (mDrawCommand == null) { // Layout was not created during the measure pass, must be Boring, create it now mDrawCommand = new DrawTextLayout(new BoringLayout( @@ -190,6 +191,7 @@ import com.facebook.react.uimanager.annotations.ReactProp; mSpacingAdd, mBoringLayoutMetrics, INCLUDE_PADDING)); + updateNodeRegion = true; } Spacing padding = getPadding(); @@ -212,6 +214,13 @@ import com.facebook.react.uimanager.annotations.ReactProp; clipBottom); stateBuilder.addDrawCommand(mDrawCommand); + if (updateNodeRegion) { + NodeRegion nodeRegion = getNodeRegion(); + if (nodeRegion instanceof TextNodeRegion) { + ((TextNodeRegion) nodeRegion).setLayout(mDrawCommand.getLayout()); + } + } + performCollectAttachDetachListeners(stateBuilder); } @@ -240,12 +249,16 @@ import com.facebook.react.uimanager.annotations.ReactProp; float right, float bottom, boolean isVirtual) { + + NodeRegion nodeRegion = getNodeRegion(); if (mDrawCommand == null) { - super.updateNodeRegion(left, top, right, bottom, isVirtual); + if (nodeRegion.mLeft != left || nodeRegion.mTop != top || nodeRegion.mRight != right || + nodeRegion.mBottom != bottom || nodeRegion.mIsVirtual != isVirtual) { + setNodeRegion(new TextNodeRegion(left, top, right, bottom, getReactTag(), isVirtual, null)); + } return; } - NodeRegion nodeRegion = getNodeRegion(); Layout layout = null; if (nodeRegion instanceof TextNodeRegion) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/flat/TextNodeRegion.java b/ReactAndroid/src/main/java/com/facebook/react/flat/TextNodeRegion.java index ab2290d53..8c9821673 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/flat/TextNodeRegion.java +++ b/ReactAndroid/src/main/java/com/facebook/react/flat/TextNodeRegion.java @@ -9,11 +9,13 @@ package com.facebook.react.flat; +import javax.annotation.Nullable; + import android.text.Layout; import android.text.Spanned; /* package */ final class TextNodeRegion extends NodeRegion { - private final Layout mLayout; + private @Nullable Layout mLayout; /* package */ TextNodeRegion( float left, @@ -22,29 +24,35 @@ import android.text.Spanned; float bottom, int tag, boolean isVirtual, - Layout layout) { + @Nullable Layout layout) { super(left, top, right, bottom, tag, isVirtual); mLayout = layout; } - /* package */ Layout getLayout() { + public void setLayout(Layout layout) { + mLayout = layout; + } + + /* package */ @Nullable Layout getLayout() { return mLayout; } /* package */ int getReactTag(float touchX, float touchY) { - int y = Math.round(touchY - mTop); - if (y >= mLayout.getLineTop(0) && y < mLayout.getLineBottom(mLayout.getLineCount() - 1)) { - float x = Math.round(touchX - mLeft); - int line = mLayout.getLineForVertical(y); + if (mLayout != null) { + int y = Math.round(touchY - mTop); + if (y >= mLayout.getLineTop(0) && y < mLayout.getLineBottom(mLayout.getLineCount() - 1)) { + float x = Math.round(touchX - mLeft); + int line = mLayout.getLineForVertical(y); - if (mLayout.getLineLeft(line) <= x && x <= mLayout.getLineRight(line)) { - int off = mLayout.getOffsetForHorizontal(line, x); + if (mLayout.getLineLeft(line) <= x && x <= mLayout.getLineRight(line)) { + int off = mLayout.getOffsetForHorizontal(line, x); - Spanned text = (Spanned) mLayout.getText(); - RCTRawText[] link = text.getSpans(off, off, RCTRawText.class); + Spanned text = (Spanned) mLayout.getText(); + RCTRawText[] link = text.getSpans(off, off, RCTRawText.class); - if (link.length != 0) { - return link[0].getReactTag(); + if (link.length != 0) { + return link[0].getReactTag(); + } } } }