Allow overriding EditText construction in ReactTextInputShadowNode

[mchowning]

Summary

This PR makes it possible for subclasses of ReactTextInputShadowNode to control the construction of the "dummy" EditText instance that ReactTextInputShadowNode internally uses to determine the expected height of the view. This PR does not change the default behavior, it just opens up that default to being overriden.

This is useful in the case of custom views that have different behavior from a "default" EditText instance (new EditText(context)). For example, it might have a different style applied.

As a side benefit, this change also makes it easy to have subclasses not apply the default theme, which can allow the custom view to avoid a longstanding crash issue (#17530).

Changelog

[Android] [Added] - Allow overriding EditText construction in ReactTextInputShadowNode

Test Plan

All tests pass.

[mchowning]

The iOS CI failures don't appear to be related to the changes in this PR.

[JoshuaGross]

I guess this seems fine, since it doesn't change any default behavior. Do you have examples of how you'd actually use this to override the default theme?

[facebook-github-bot]

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

[mchowning]

Do you have examples of how you'd actually use this to override the default theme?

The simplest example would be to just create an EditText with a null AttributeSet and no (0) default style attributes:

@Override
 protected EditText createDummyEditText() {
   return new EditText(getThemedContext(), null, 0);
 }

This is actually a sufficient change to avoid the crash in #17530, and it also avoids applying padding based on the default EditText background, which varies by device manufacturer.

Another example would be that you have a customized EditText with various attributes (custom style, padding, etc.) that you are applying either in xml or in a subclass, and you would like the ShadowNode's measurement of the view to account for those attributes. In that case, you could do something like:

@Override
 protected EditText createDummyEditText() {
   return LayoutInflater.from(reactContext)
                  .inflate(R.layout.my_edit_text, ...);
 }

[JoshuaGross]

Let's call this something like createInternalEditText or createEditTextForMeasure since this is becoming a public API.

[JoshuaGross]

Let's also add a javadoc here to document what this is for and how it can be used.

[JoshuaGross]

See comments; I'm mostly fine with this but if this is enabling someone to manually fix a crash, I w wonder if there's an additional, permanent solution to that crash. Requiring that everyone implement a custom TextInput component to avoid a crash doesn't seem great.

[mchowning]

if this is enabling someone to manually fix a crash, I w wonder if there's an additional, permanent solution to that crash. Requiring that everyone implement a custom TextInput component to avoid a crash doesn't seem great.

Agreed. Unfortunately, this can only be used to avoid the crash if you have a custom view that does not rely on the default EditText background. Admittedly, there is probably a decent chance that a custom EditText view will have a different background, but it still makes sense for the default behavior to measure based on the default EditText background. For that reason, I don't consider this change as including a general fix for that crash, I just was pointing out that in a specific scenario (custom view that doesn't use default EditText background), it does allow the crash to be avoided for that view. The actual fix for that crash will need to fix it even when the default drawable is applied because, among other things, that is the situation for the TextInput component.

If that crash fix were the only reason behind this change, I would be hesitant to suggest this change. I think it is the fact that this change allows the subclass to alter the behavior of the internal EditText in other ways is what makes this worthwhile. The ability to fix the crash is just a nice byproduct.

[facebook-github-bot]

@JoshuaGross has imported 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 @mchowning in a5b5d1a.

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