From 1aa9444511980f6a1935815bf905655414360f0f Mon Sep 17 00:00:00 2001 From: Gabriel Donadel Dall'Agnol Date: Sun, 28 Aug 2022 11:24:58 -0300 Subject: [PATCH 1/5] feat: Add id prop to View component --- Libraries/Components/View/View.js | 3 +++ Libraries/Components/View/ViewPropTypes.js | 9 +++++++++ 2 files changed, 12 insertions(+) diff --git a/Libraries/Components/View/View.js b/Libraries/Components/View/View.js index 97020938e6d01b..e069d8289a7e06 100644 --- a/Libraries/Components/View/View.js +++ b/Libraries/Components/View/View.js @@ -36,7 +36,9 @@ const View: React.AbstractComponent< accessibilityRole, 'aria-hidden': ariaHidden, focusable, + id, importantForAccessibility, + nativeID, pointerEvents, role, style, @@ -161,6 +163,7 @@ const View: React.AbstractComponent< ? 'no-hide-descendants' : importantForAccessibility } + nativeID={id ?? nativeID} {...restWithDefaultProps} style={style} pointerEvents={newPointerEvents} diff --git a/Libraries/Components/View/ViewPropTypes.js b/Libraries/Components/View/ViewPropTypes.js index c6cb8655d50532..7293fde9169304 100644 --- a/Libraries/Components/View/ViewPropTypes.js +++ b/Libraries/Components/View/ViewPropTypes.js @@ -540,6 +540,15 @@ export type ViewProps = $ReadOnly<{| */ collapsable?: ?boolean, + /** + * Used to locate this view from native classes. + * + * > This disables the 'layout-only view removal' optimization for this view! + * + * See https://reactnative.dev/docs/view#id + */ + id?: ?string, + /** * Used to locate this view in end-to-end tests. * From 3ff0c2c27c197b2b7faebcd59f36f3dd7dd08b0f Mon Sep 17 00:00:00 2001 From: Gabriel Donadel Dall'Agnol Date: Sun, 28 Aug 2022 11:34:46 -0300 Subject: [PATCH 2/5] feat: Add id prop to TouchableWithoutFeedback component --- Libraries/Components/Touchable/TouchableWithoutFeedback.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Libraries/Components/Touchable/TouchableWithoutFeedback.js b/Libraries/Components/Touchable/TouchableWithoutFeedback.js index ab2d844253b717..706e6aaeee9cab 100755 --- a/Libraries/Components/Touchable/TouchableWithoutFeedback.js +++ b/Libraries/Components/Touchable/TouchableWithoutFeedback.js @@ -65,6 +65,7 @@ type Props = $ReadOnly<{| disabled?: ?boolean, focusable?: ?boolean, hitSlop?: ?EdgeInsetsProp, + id?: ?string, importantForAccessibility?: ?('auto' | 'yes' | 'no' | 'no-hide-descendants'), nativeID?: ?string, onAccessibilityAction?: ?(event: AccessibilityActionEvent) => mixed, @@ -102,7 +103,6 @@ const PASSTHROUGH_PROPS = [ 'accessibilityViewIsModal', 'hitSlop', 'importantForAccessibility', - 'nativeID', 'onAccessibilityAction', 'onBlur', 'onFocus', @@ -168,6 +168,7 @@ class TouchableWithoutFeedback extends React.Component { ariaLive === 'off' ? 'none' : ariaLive ?? this.props.accessibilityLiveRegion, + nativeID: this.props.id ?? this.props.nativeID, }; for (const prop of PASSTHROUGH_PROPS) { if (this.props[prop] !== undefined) { From fc4233db1963a4949f2ae50249db9a67ce6e12e8 Mon Sep 17 00:00:00 2001 From: Gabriel Donadel Dall'Agnol Date: Sun, 28 Aug 2022 11:44:35 -0300 Subject: [PATCH 3/5] feat: Add id prop to Text component --- Libraries/Text/Text.js | 4 ++++ Libraries/Text/TextProps.js | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/Libraries/Text/Text.js b/Libraries/Text/Text.js index eb1bb5f2341a9f..4e2cd697341b29 100644 --- a/Libraries/Text/Text.js +++ b/Libraries/Text/Text.js @@ -40,6 +40,8 @@ const Text: React.AbstractComponent< 'aria-expanded': ariaExpanded, 'aria-selected': ariaSelected, ellipsizeMode, + id, + nativeID, onLongPress, onPress, onPressIn, @@ -205,6 +207,7 @@ const Text: React.AbstractComponent< isHighlighted={isHighlighted} isPressable={isPressable} selectable={_selectable} + nativeID={id ?? nativeID} numberOfLines={numberOfLines} selectionColor={selectionColor} style={style} @@ -222,6 +225,7 @@ const Text: React.AbstractComponent< allowFontScaling={allowFontScaling !== false} ellipsizeMode={ellipsizeMode ?? 'tail'} isHighlighted={isHighlighted} + nativeID={id ?? nativeID} numberOfLines={numberOfLines} selectionColor={selectionColor} style={style} diff --git a/Libraries/Text/TextProps.js b/Libraries/Text/TextProps.js index 5ccb9f15e10383..1453f9c188e4b7 100644 --- a/Libraries/Text/TextProps.js +++ b/Libraries/Text/TextProps.js @@ -98,6 +98,13 @@ export type TextProps = $ReadOnly<{| */ ellipsizeMode?: ?('clip' | 'head' | 'middle' | 'tail'), + /** + * Used to locate this view from native code. + * + * See https://reactnative.dev/docs/text#nativeid + */ + id?: ?string, + /** * Specifies largest possible scale a font can reach when `allowFontScaling` is enabled. * Possible values: From dc2bbfd0fa1781d3b643d575c56cb9ddfff3b474 Mon Sep 17 00:00:00 2001 From: Gabriel Donadel Dall'Agnol Date: Sun, 28 Aug 2022 11:58:36 -0300 Subject: [PATCH 4/5] test: Add id prop android tests --- .../com/facebook/react/tests/IdTestCase.java | 89 +++++++++++++++++++ .../src/androidTest/js/IdTestModule.js | 76 ++++++++++++++++ ReactAndroid/src/androidTest/js/TestApps.js | 4 + 3 files changed, 169 insertions(+) create mode 100644 ReactAndroid/src/androidTest/java/com/facebook/react/tests/IdTestCase.java create mode 100644 ReactAndroid/src/androidTest/js/IdTestModule.js diff --git a/ReactAndroid/src/androidTest/java/com/facebook/react/tests/IdTestCase.java b/ReactAndroid/src/androidTest/java/com/facebook/react/tests/IdTestCase.java new file mode 100644 index 00000000000000..665635f3275974 --- /dev/null +++ b/ReactAndroid/src/androidTest/java/com/facebook/react/tests/IdTestCase.java @@ -0,0 +1,89 @@ +/* + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +package com.facebook.react.tests; + +import android.view.View; +import com.facebook.react.testing.ReactAppInstrumentationTestCase; +import com.facebook.react.uimanager.util.ReactFindViewUtil; +import java.util.Arrays; +import java.util.List; + +/** + * Tests that the 'id' property can be set on various views. The 'id' property is used + * to reference react managed views from native code. + */ +public class IdTestCase extends ReactAppInstrumentationTestCase { + + @Override + protected String getReactApplicationKeyUnderTest() { + return "IdTestApp"; + } + + private final List viewTags = + Arrays.asList( + "Image", + "Text", + "TouchableBounce", + "TouchableHighlight", + "TouchableOpacity", + "TouchableWithoutFeedback", + "TextInput", + "View"); + + private boolean mViewFound; + + @Override + protected void setUp() throws Exception { + mViewFound = false; + ReactFindViewUtil.addViewListener( + new ReactFindViewUtil.OnViewFoundListener() { + @Override + public String getNativeId() { + return viewTags.get(0); + } + + @Override + public void onViewFound(View view) { + mViewFound = true; + } + }); + super.setUp(); + } + + public void testPropertyIsSetForViews() { + for (String nativeId : viewTags) { + View viewWithTag = ReactFindViewUtil.findView(getActivity().getRootView(), nativeId); + assertNotNull( + "View with id " + nativeId + " was not found. Check IdTestModule.js.", + viewWithTag); + } + } + + public void testViewListener() { + assertTrue("OnViewFound callback was never invoked", mViewFound); + } + + public void testFindView() { + mViewFound = false; + ReactFindViewUtil.findView( + getActivity().getRootView(), + new ReactFindViewUtil.OnViewFoundListener() { + @Override + public String getNativeId() { + return viewTags.get(0); + } + + @Override + public void onViewFound(View view) { + mViewFound = true; + } + }); + assertTrue( + "OnViewFound callback should have successfully been invoked synchronously", mViewFound); + } +} diff --git a/ReactAndroid/src/androidTest/js/IdTestModule.js b/ReactAndroid/src/androidTest/js/IdTestModule.js new file mode 100644 index 00000000000000..278670f35274af --- /dev/null +++ b/ReactAndroid/src/androidTest/js/IdTestModule.js @@ -0,0 +1,76 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + * @flow strict-local + */ + +'use strict'; + +const React = require('react'); +const TouchableBounce = require('react-native/Libraries/Components/Touchable/TouchableBounce'); + +const { + Image, + StyleSheet, + Text, + TextInput, + TouchableHighlight, + TouchableOpacity, + TouchableWithoutFeedback, + View, +} = require('react-native'); + +/** + * All the views implemented on Android, each with the id property set. + * We test that: + * - The app renders fine + * - The id property is passed to the native views via the nativeID property + */ +class IdTestApp extends React.Component<{...}> { + render(): React.Node { + const uri = + 'data:image/gif;base64,' + + 'R0lGODdhMAAwAPAAAAAAAP///ywAAAAAMAAwAAAC8IyPqcvt3wCcDkiLc7C0qwyGHhSWpjQu5yqmCYsapy' + + 'uvUUlvONmOZtfzgFzByTB10QgxOR0TqBQejhRNzOfkVJ+5YiUqrXF5Y5lKh/DeuNcP5yLWGsEbtLiOSpa/' + + 'TPg7JpJHxyendzWTBfX0cxOnKPjgBzi4diinWGdkF8kjdfnycQZXZeYGejmJlZeGl9i2icVqaNVailT6F5' + + 'iJ90m6mvuTS4OK05M0vDk0Q4XUtwvKOzrcd3iq9uisF81M1OIcR7lEewwcLp7tuNNkM3uNna3F2JQFo97V' + + 'riy/Xl4/f1cf5VWzXyym7PHhhx4dbgYKAAA7'; + return ( + + + text + + + TouchableBounce + + + TouchableHighlight + + + TouchableOpacity + + + + TouchableWithoutFeedback + + + + + ); + } +} + +const styles = StyleSheet.create({ + base: { + width: 150, + height: 50, + }, +}); + +module.exports = { + IdTestApp, +}; diff --git a/ReactAndroid/src/androidTest/js/TestApps.js b/ReactAndroid/src/androidTest/js/TestApps.js index 4659e2c4f86322..208b6cdf91eb0a 100644 --- a/ReactAndroid/src/androidTest/js/TestApps.js +++ b/ReactAndroid/src/androidTest/js/TestApps.js @@ -48,6 +48,10 @@ const apps = [ component: () => require('./ScrollViewTestModule').HorizontalScrollViewTestApp, }, + { + appKey: 'IdTestApp', + component: () => require('./IdTestModule').IdTestApp, + }, { appKey: 'ImageOverlayColorTestApp', component: () => require('./ImageOverlayColorTestApp'), From a3ecb80466427821045aed889c3bc8ff57d410f7 Mon Sep 17 00:00:00 2001 From: Gabriel Donadel Dall'Agnol Date: Mon, 5 Sep 2022 09:06:35 -0300 Subject: [PATCH 5/5] chore: Update id type definition --- Libraries/Components/Touchable/TouchableWithoutFeedback.js | 2 +- Libraries/Components/View/ViewPropTypes.js | 2 +- Libraries/Text/TextProps.js | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Libraries/Components/Touchable/TouchableWithoutFeedback.js b/Libraries/Components/Touchable/TouchableWithoutFeedback.js index 706e6aaeee9cab..9af740f7c95bc8 100755 --- a/Libraries/Components/Touchable/TouchableWithoutFeedback.js +++ b/Libraries/Components/Touchable/TouchableWithoutFeedback.js @@ -65,7 +65,7 @@ type Props = $ReadOnly<{| disabled?: ?boolean, focusable?: ?boolean, hitSlop?: ?EdgeInsetsProp, - id?: ?string, + id?: string, importantForAccessibility?: ?('auto' | 'yes' | 'no' | 'no-hide-descendants'), nativeID?: ?string, onAccessibilityAction?: ?(event: AccessibilityActionEvent) => mixed, diff --git a/Libraries/Components/View/ViewPropTypes.js b/Libraries/Components/View/ViewPropTypes.js index 7293fde9169304..30c0426e984624 100644 --- a/Libraries/Components/View/ViewPropTypes.js +++ b/Libraries/Components/View/ViewPropTypes.js @@ -547,7 +547,7 @@ export type ViewProps = $ReadOnly<{| * * See https://reactnative.dev/docs/view#id */ - id?: ?string, + id?: string, /** * Used to locate this view in end-to-end tests. diff --git a/Libraries/Text/TextProps.js b/Libraries/Text/TextProps.js index 1453f9c188e4b7..b017ae0b54a7cc 100644 --- a/Libraries/Text/TextProps.js +++ b/Libraries/Text/TextProps.js @@ -103,7 +103,7 @@ export type TextProps = $ReadOnly<{| * * See https://reactnative.dev/docs/text#nativeid */ - id?: ?string, + id?: string, /** * Specifies largest possible scale a font can reach when `allowFontScaling` is enabled.