From cd0319431749c21aa38a911cb093ed193caf67d1 Mon Sep 17 00:00:00 2001 From: jsdevel Date: Fri, 16 Sep 2016 03:42:03 -0700 Subject: [PATCH] Adding support for resource-id. * This closes #9777. --- .../react/XReactInstanceManagerImpl.java | 3 +- .../react/touch/JSResponderHandler.java | 4 +- .../react/uimanager/BaseViewManager.java | 35 +++++ .../uimanager/NativeViewHierarchyManager.java | 17 ++- .../react/uimanager/TouchTargetHelper.java | 8 +- .../react/uimanager/events/Event.java | 2 + .../react/views/text/ReactTextView.java | 4 +- .../react/uimanager/BaseViewManagerTest.java | 141 ++++++++++++++++++ 8 files changed, 201 insertions(+), 13 deletions(-) create mode 100644 ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java diff --git a/ReactAndroid/src/main/java/com/facebook/react/XReactInstanceManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/XReactInstanceManagerImpl.java index 239e2e5b3043b8..1ff2bff7b082fd 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/XReactInstanceManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/XReactInstanceManagerImpl.java @@ -86,6 +86,7 @@ import static com.facebook.react.bridge.ReactMarkerConstants.RUN_JS_BUNDLE_START; import static com.facebook.react.bridge.ReactMarkerConstants.SETUP_REACT_CONTEXT_END; import static com.facebook.react.bridge.ReactMarkerConstants.SETUP_REACT_CONTEXT_START; +import static com.facebook.react.uimanager.BaseViewManager.getOriginalReactTag; import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE; /** @@ -811,7 +812,7 @@ private void detachViewFromInstance( CatalystInstance catalystInstance) { UiThreadUtil.assertOnUiThread(); catalystInstance.getJSModule(AppRegistry.class) - .unmountApplicationComponentAtRootTag(rootView.getId()); + .unmountApplicationComponentAtRootTag(getOriginalReactTag(rootView)); } private void tearDownReactContext(ReactContext reactContext) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/touch/JSResponderHandler.java b/ReactAndroid/src/main/java/com/facebook/react/touch/JSResponderHandler.java index 7140e2c303fea8..777ae432a09296 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/touch/JSResponderHandler.java +++ b/ReactAndroid/src/main/java/com/facebook/react/touch/JSResponderHandler.java @@ -15,6 +15,8 @@ import android.view.ViewGroup; import android.view.ViewParent; +import static com.facebook.react.uimanager.BaseViewManager.getOriginalReactTag; + /** * This class coordinates JSResponder commands for {@link UIManagerModule}. It should be set as * OnInterceptTouchEventListener for all newly created native views that implements @@ -70,7 +72,7 @@ public boolean onInterceptTouchEvent(ViewGroup v, MotionEvent event) { // Therefore since "UP" event is the last event in a gesture, we should just let it reach the // original target that is a child view of {@param v}. // http://developer.android.com/reference/android/view/ViewGroup.html#onInterceptTouchEvent(android.view.MotionEvent) - return v.getId() == currentJSResponder; + return getOriginalReactTag(v) == currentJSResponder; } return false; } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java index 815bad0faddcc5..8c811dfa16e26c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManager.java @@ -8,7 +8,9 @@ import android.view.ViewGroup; import com.facebook.react.bridge.ReadableArray; +import com.facebook.react.common.annotations.VisibleForTesting; import com.facebook.react.uimanager.annotations.ReactProp; +import java.util.concurrent.ConcurrentHashMap; /** * Base class that should be suitable for the majority of subclasses of {@link ViewManager}. @@ -35,6 +37,9 @@ public abstract class BaseViewManager TEST_IDS = new ConcurrentHashMap<>(); + private static final ConcurrentHashMap JS_IDS = new ConcurrentHashMap<>(); + /** * Used to locate views in end-to-end (UI) tests. */ @@ -84,6 +89,14 @@ public void setRenderToHardwareTexture(T view, boolean useHWTexture) { @ReactProp(name = PROP_TEST_ID) public void setTestId(T view, String testId) { + if (!TEST_IDS.containsKey(testId)) { + TEST_IDS.put(testId, view.getResources().getIdentifier(testId, "id", view.getContext().getPackageName())); + } + int mappedTestId = TEST_IDS.get(testId); + if (mappedTestId != 0) { + JS_IDS.put(System.identityHashCode(view), view.getId()); + view.setId(mappedTestId); + } view.setTag(testId); } @@ -153,6 +166,28 @@ public void setAccessibilityLiveRegion(T view, String liveRegion) { } } + /** + * Returns the tag originally given by the javascript bridge when this view was created prior to + * it being potentially overwritten by {@link #setTestId}. + * + * @param view + * @param + * @return + */ + public static int getOriginalReactTag(T view) { + Integer idFromJs = JS_IDS.get(System.identityHashCode(view)); + return idFromJs != null ? idFromJs.intValue() : view.getId(); + } + + /** + * Used internally to clear the state of test ids. + */ + @VisibleForTesting + static void resetTestState() { + JS_IDS.clear(); + TEST_IDS.clear(); + } + private static void setTransformProperty(View view, ReadableArray transforms) { TransformHelper.processTransform(transforms, sTransformDecompositionArray); MatrixMathHelper.decomposeMatrix(sTransformDecompositionArray, sMatrixDecompositionContext); diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java index 3d6ad539a58129..538cff72886b87 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java @@ -39,6 +39,8 @@ import javax.annotation.Nullable; import javax.annotation.concurrent.NotThreadSafe; +import static com.facebook.react.uimanager.BaseViewManager.getOriginalReactTag; + /** * Delegate of {@link UIManagerModule} that owns the native view hierarchy and mapping between * native view names used in JS and corresponding instances of {@link ViewManager}. The @@ -358,7 +360,7 @@ public void manageChildren( if (mLayoutAnimationEnabled && mLayoutAnimator.shouldAnimateLayout(viewToRemove) && - arrayContains(tagsToDelete, viewToRemove.getId())) { + arrayContains(tagsToDelete, getOriginalReactTag(viewToRemove))) { // The view will be removed and dropped by the 'delete' layout animation // instead, so do nothing } else { @@ -512,24 +514,25 @@ protected final void addRootViewGroup( */ protected void dropView(View view) { UiThreadUtil.assertOnUiThread(); - if (!mRootTags.get(view.getId())) { + int originalReactTag = getOriginalReactTag(view); + if (!mRootTags.get(originalReactTag)) { // For non-root views we notify viewmanager with {@link ViewManager#onDropInstance} - resolveViewManager(view.getId()).onDropViewInstance(view); + resolveViewManager(originalReactTag).onDropViewInstance(view); } - ViewManager viewManager = mTagsToViewManagers.get(view.getId()); + ViewManager viewManager = mTagsToViewManagers.get(originalReactTag); if (view instanceof ViewGroup && viewManager instanceof ViewGroupManager) { ViewGroup viewGroup = (ViewGroup) view; ViewGroupManager viewGroupManager = (ViewGroupManager) viewManager; for (int i = viewGroupManager.getChildCount(viewGroup) - 1; i >= 0; i--) { View child = viewGroupManager.getChildAt(viewGroup, i); - if (mTagsToViews.get(child.getId()) != null) { + if (mTagsToViews.get(getOriginalReactTag(child)) != null) { dropView(child); } } viewGroupManager.removeAllViews(viewGroup); } - mTagsToViews.remove(view.getId()); - mTagsToViewManagers.remove(view.getId()); + mTagsToViews.remove(originalReactTag); + mTagsToViewManagers.remove(originalReactTag); } public void removeRootView(int rootViewTag) { diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java index 4ef74fafc941f5..a98aa6130329be 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/TouchTargetHelper.java @@ -22,6 +22,8 @@ import com.facebook.react.bridge.UiThreadUtil; import com.facebook.react.touch.ReactHitSlopView; +import static com.facebook.react.uimanager.BaseViewManager.getOriginalReactTag; + /** * Class responsible for identifying which react view should handle a given {@link MotionEvent}. * It uses the event coordinates to traverse the view hierarchy and return a suitable view. @@ -87,7 +89,7 @@ public static int findTargetTagAndCoordinatesForTouch( float[] viewCoords, @Nullable int[] nativeViewTag) { UiThreadUtil.assertOnUiThread(); - int targetTag = viewGroup.getId(); + int targetTag = getOriginalReactTag(viewGroup); // Store eventCoords in array so that they are modified to be relative to the targetView found. viewCoords[0] = eventX; viewCoords[1] = eventY; @@ -224,7 +226,7 @@ private static boolean isTransformedTouchPointInView( // ViewGroup). if (view instanceof ReactCompoundView) { int reactTag = ((ReactCompoundView)view).reactTagForTouch(eventCoords[0], eventCoords[1]); - if (reactTag != view.getId()) { + if (reactTag != getOriginalReactTag(view)) { // make sure we exclude the View itself because of the PointerEvents.BOX_NONE return view; } @@ -256,7 +258,7 @@ private static int getTouchTargetForView(View targetView, float eventX, float ev // {@link #findTouchTargetView()}. return ((ReactCompoundView) targetView).reactTagForTouch(eventX, eventY); } - return targetView.getId(); + return getOriginalReactTag(targetView); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java index 4da9ed777d3758..af639908d68d21 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/events/Event.java @@ -44,6 +44,8 @@ protected void init(int viewTag) { } /** + * TODO: Determine if this can be affected by testID. + * * @return the view id for the view that generated this event */ public final int getViewTag() { diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java index 50828a80ba6818..cbf8b625ad8d61 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java @@ -26,6 +26,8 @@ import com.facebook.react.uimanager.ViewDefaults; import com.facebook.react.views.view.ReactViewBackgroundDrawable; +import static com.facebook.react.uimanager.BaseViewManager.getOriginalReactTag; + public class ReactTextView extends TextView implements ReactCompoundView { private static final ViewGroup.LayoutParams EMPTY_LAYOUT_PARAMS = @@ -74,7 +76,7 @@ public void setText(ReactTextUpdate update) { @Override public int reactTagForTouch(float touchX, float touchY) { Spanned text = (Spanned) getText(); - int target = getId(); + int target = getOriginalReactTag(this); int x = (int) touchX; int y = (int) touchY; diff --git a/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java b/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java new file mode 100644 index 00000000000000..e57ed1f5f66987 --- /dev/null +++ b/ReactAndroid/src/test/java/com/facebook/react/uimanager/BaseViewManagerTest.java @@ -0,0 +1,141 @@ +/** + * Copyright (c) 2015-present, Facebook, Inc. + * All rights reserved. + * + * This source code is licensed under the BSD-style license found in the + * LICENSE file in the root directory of this source tree. An additional grant + * of patent rights can be found in the PATENTS file in the same directory. + */ + +package com.facebook.react.uimanager; + +import android.content.Context; +import android.content.res.Resources; +import android.view.View; + +import com.facebook.csslayout.Spacing; + +import org.junit.After; +import org.junit.Before; +import org.junit.FixMethodOrder; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.MethodSorters; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.powermock.core.classloader.annotations.PowerMockIgnore; +import org.powermock.core.classloader.annotations.PrepareForTest; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.annotation.Config; + +import static org.junit.Assert.assertEquals; +import static org.mockito.AdditionalMatchers.not; +import static org.mockito.Matchers.anyFloat; +import static org.mockito.Matchers.anyInt; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; + +@Config(manifest= Config.NONE) +@RunWith(RobolectricTestRunner.class) +@PowerMockIgnore({"org.mockito.*", "org.robolectric.*", "android.*"}) +public class BaseViewManagerTest { + + @Mock + Resources resources; + @Mock + Context context; + @Mock + private View view; + private BaseViewManager sut; + + private final String testID = "some-test-id"; + private final int mappedTestID = 23457897; + private final int originalJsTag = 5; + private final String myPackage = "com.myApp"; + + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + when(view.getContext()).thenReturn(context); + when(view.getResources()).thenReturn(resources); + when(view.getId()).thenReturn(originalJsTag); + when(resources.getIdentifier(eq(testID), eq("id"), eq(myPackage))).thenReturn(mappedTestID); + when(resources.getIdentifier(eq(testID), eq("id"), not(eq(myPackage)))).thenReturn(0); + sut = new ViewManagerStub(); + } + + @After + public void teardown() { + BaseViewManager.resetTestState(); + } + + @Test + public void testSetTestId_should_always_call_setTag() { + String expectedTestID1 = "asdfasdf1"; + String expectedTestID2 = "asdfasdf2"; + when(context.getPackageName()).thenReturn("com.foo"); + sut.setTestId(view, expectedTestID1); + sut.setTestId(view, expectedTestID2); + verify(view).setTag(expectedTestID1); + verify(view).setTag(expectedTestID2); + } + + @Test + public void testSetTestId_should_not_set_the_mapped_testID_on_the_view_when_a_resource_id_is_not_found() { + when(context.getPackageName()).thenReturn("com.foo"); + sut.setTestId(view, testID); + verify(view, never()).setId(anyInt()); + } + + @Test + public void testSetTestId_should_set_the_mapped_testID_on_the_view_when_a_resource_id_is_found() { + when(context.getPackageName()).thenReturn(myPackage); + sut.setTestId(view, testID); + verify(view).setId(mappedTestID); + } + + @Test + public void getOriginalReactTag_should_return_the_original_tag_set_by_js_if_no_testID_has_been_set() { + assertEquals("The original JS tag was not returned", BaseViewManager.getOriginalReactTag(view), originalJsTag); + } + + @Test + public void getOriginalReactTag_should_return_the_original_tag_set_by_js_if_testID_has_been_set() { + when(context.getPackageName()).thenReturn(myPackage); + sut.setTestId(view, testID); + verify(view).setId(mappedTestID); + assertEquals("The original JS tag was not returned", BaseViewManager.getOriginalReactTag(view), originalJsTag); + } + + private static class ViewManagerStub extends BaseViewManager { + @Override + public String getName() { + return null; + } + + @Override + public LayoutShadowNode createShadowNodeInstance() { + return null; + } + + @Override + public Class getShadowNodeClass() { + return null; + } + + @Override + protected View createViewInstance(ThemedReactContext reactContext) { + return null; + } + + @Override + public void updateExtraData(View root, Object extraData) { + + } + } +}