From c8ed2dbbb287deed05a8782fb8665c1edf45bbac Mon Sep 17 00:00:00 2001
From: Vojtech Novak <vonovak@gmail.com>
Date: Mon, 18 May 2020 18:16:22 -0700
Subject: [PATCH] get ripple drawables by id (#28600)

Summary:
While working on recent PRs regarding ripple radius in TouchableNativeFeedbaack and ripple support in Pressable I noticed `ReactDrawableHelper` uses a [discouraged](https://developer.android.com/reference/android/content/res/Resources#getIdentifier(java.lang.String,%20java.lang.String,%20java.lang.String)) way to obtain resources.

The attribute names (strings) `'selectableItemBackground'` and `'selectableItemBackgroundBorderless'` are used here

https://github.com/facebook/react-native/blob/4a48b021d63a474f1570e92616988384957d4273/Libraries/Components/Touchable/TouchableNativeFeedback.js#L105

And passed to `context.getResources().getIdentifier()` in `ReactDrawableHelper`. Since we know the attribute names beforehand I figured we can obtain the resources by id (fast) instead of by name (slow). I made it so that the slow code path is taken in case the attribute name does not match what is expected, as a fallback.

Note that I did not do any measurement of the effect of this, I'm just offering this as a PR. You'll notice that this PR relies on the fact that the string in JS is the same as the string in Java (it is duplicated). While I could export the strings from Java and use them in JS, I wasn't sure where to export them. But note that even before, the JS code depended on the `'selectableItemBackground'` and `'selectableItemBackgroundBorderless'` strings to exist on the native side, in the android SDK, I just made the dependency explicit.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Changed] - get ripple drawables by id
Pull Request resolved: https://github.com/facebook/react-native/pull/28600

Test Plan: tested manually in RNTester

Differential Revision: D21241773

Pulled By: shergin

fbshipit-source-id: 1b8314f99616095cb6ed557c62095cf3200f53b6
---
 .../react/views/view/ReactDrawableHelper.java | 24 ++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java
index add5f38812baa0..7d93ea2f0ab517 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java
@@ -17,6 +17,7 @@
 import android.os.Build;
 import android.util.TypedValue;
 import androidx.annotation.Nullable;
+
 import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
 import com.facebook.react.bridge.ReadableMap;
 import com.facebook.react.bridge.SoftAssertions;
@@ -37,15 +38,10 @@ public static Drawable createDrawableFromJSDescription(
     String type = drawableDescriptionDict.getString("type");
     if ("ThemeAttrAndroid".equals(type)) {
       String attr = drawableDescriptionDict.getString("attribute");
-      SoftAssertions.assertNotNull(attr);
-      int attrID = context.getResources().getIdentifier(attr, "attr", "android");
-      if (attrID == 0) {
-        throw new JSApplicationIllegalArgumentException(
-            "Attribute " + attr + " couldn't be found in the resource list");
-      }
-      if (!context.getTheme().resolveAttribute(attrID, sResolveOutValue, true)) {
+      int attrId = getAttrId(context, attr);
+      if (!context.getTheme().resolveAttribute(attrId, sResolveOutValue, true)) {
         throw new JSApplicationIllegalArgumentException(
-            "Attribute " + attr + " couldn't be resolved into a drawable");
+            "Attribute " + attr + " with id " + attrId + " couldn't be resolved into a drawable");
       }
       Drawable drawable = getDefaultThemeDrawable(context);
       return setRadius(drawableDescriptionDict, drawable);
@@ -57,6 +53,18 @@ public static Drawable createDrawableFromJSDescription(
     }
   }
 
+  @TargetApi(Build.VERSION_CODES.LOLLIPOP)
+  private static int getAttrId(Context context, String attr) {
+    SoftAssertions.assertNotNull(attr);
+    if ("selectableItemBackground".equals(attr)) {
+      return android.R.attr.selectableItemBackground;
+    } else if ("selectableItemBackgroundBorderless".equals(attr)) {
+      return android.R.attr.selectableItemBackgroundBorderless;
+    } else {
+      return context.getResources().getIdentifier(attr, "attr", "android");
+    }
+  }
+
   private static Drawable getDefaultThemeDrawable(Context context) {
     if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
       return context.getResources().getDrawable(sResolveOutValue.resourceId, context.getTheme());