Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get ripple drawables by id #28600

Closed
wants to merge 2 commits into from
Closed

get ripple drawables by id #28600

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c6b8aeb
get ripple drawable by id if possible
vonovak Apr 6, 2020
c0215e3
include attribute id in error message
vonovak Apr 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 16 additions & 8 deletions ReactAndroid/src/main/java/com/facebook/react/views/view/ReactDrawableHelper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about if this return 0?
should we throw an exception as we did in the past?
(A comment from David Vacca @mdvacca.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdvacca The output of this is fed into context.getTheme().resolveAttribute()on line 42 and if that returns false (which it would for 0), JSApplicationIllegalArgumentException is thrown, so it's the same behavior as before.

}
}

private static Drawable getDefaultThemeDrawable(Context context) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.LOLLIPOP) {
return context.getResources().getDrawable(sResolveOutValue.resourceId, context.getTheme());
Expand Down