Skip to content

Commit c8ed2db

Browse files
vonovakfacebook-github-bot
authored andcommitted
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: #28600 Test Plan: tested manually in RNTester Differential Revision: D21241773 Pulled By: shergin fbshipit-source-id: 1b8314f99616095cb6ed557c62095cf3200f53b6
1 parent 04b8c9c commit c8ed2db

File tree

1 file changed

+16
-8
lines changed

1 file changed

+16
-8
lines changed

ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java

+16-8
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import android.os.Build;
1818
import android.util.TypedValue;
1919
import androidx.annotation.Nullable;
20+
2021
import com.facebook.react.bridge.JSApplicationIllegalArgumentException;
2122
import com.facebook.react.bridge.ReadableMap;
2223
import com.facebook.react.bridge.SoftAssertions;
@@ -37,15 +38,10 @@ public static Drawable createDrawableFromJSDescription(
3738
String type = drawableDescriptionDict.getString("type");
3839
if ("ThemeAttrAndroid".equals(type)) {
3940
String attr = drawableDescriptionDict.getString("attribute");
40-
SoftAssertions.assertNotNull(attr);
41-
int attrID = context.getResources().getIdentifier(attr, "attr", "android");
42-
if (attrID == 0) {
43-
throw new JSApplicationIllegalArgumentException(
44-
"Attribute " + attr + " couldn't be found in the resource list");
45-
}
46-
if (!context.getTheme().resolveAttribute(attrID, sResolveOutValue, true)) {
41+
int attrId = getAttrId(context, attr);
42+
if (!context.getTheme().resolveAttribute(attrId, sResolveOutValue, true)) {
4743
throw new JSApplicationIllegalArgumentException(
48-
"Attribute " + attr + " couldn't be resolved into a drawable");
44+
"Attribute " + attr + " with id " + attrId + " couldn't be resolved into a drawable");
4945
}
5046
Drawable drawable = getDefaultThemeDrawable(context);
5147
return setRadius(drawableDescriptionDict, drawable);
@@ -57,6 +53,18 @@ public static Drawable createDrawableFromJSDescription(
5753
}
5854
}
5955

56+
@TargetApi(Build.VERSION_CODES.LOLLIPOP)
57+
private static int getAttrId(Context context, String attr) {
58+
SoftAssertions.assertNotNull(attr);
59+
if ("selectableItemBackground".equals(attr)) {
60+
return android.R.attr.selectableItemBackground;
61+
} else if ("selectableItemBackgroundBorderless".equals(attr)) {
62+
return android.R.attr.selectableItemBackgroundBorderless;
63+
} else {
64+
return context.getResources().getIdentifier(attr, "attr", "android");
65+
}
66+
}
67+
6068
private static Drawable getDefaultThemeDrawable(Context context) {
6169
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
6270
return context.getResources().getDrawable(sResolveOutValue.resourceId, context.getTheme());

0 commit comments

Comments
 (0)