get ripple drawables by id

[vonovak]

Summary

While working on recent PRs regarding ripple radius in TouchableNativeFeedbaack and ripple support in Pressable I noticed ReactDrawableHelper uses a discouraged way to obtain resources.

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

react-native/Libraries/Components/Touchable/TouchableNativeFeedback.js

Line 105 in 4a48b02

static SelectableBackground: (

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

[Android] [Changed] - get ripple drawables by id

Test Plan

tested manually in RNTester

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	809,568	0

Base commit: d7299e8

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	6,739,209	116
android	hermes	armeabi-v7a	6,404,263	133
android	hermes	x86	7,121,368	112
android	hermes	x86_64	7,011,924	115
android	jsc	arm64-v8a	8,907,549	109
android	jsc	armeabi-v7a	8,564,994	110
android	jsc	x86	8,732,834	127
android	jsc	x86_64	9,309,024	116

Base commit: d7299e8

[facebook-github-bot]

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[shergin]

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

[vonovak]

@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.

[facebook-github-bot]

@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[react-native-bot]

This pull request was successfully merged by @vonovak in c8ed2db.

^{When will my fix make it into a release? | Upcoming Releases}