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.

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

allow custom maybeSetText logic for ReactEditText subclasses #24927

Closed
wants to merge 1 commit into from

Conversation

vonovak
Copy link
Contributor

@vonovak vonovak commented May 17, 2019

Summary

We're working on a custom EditText that supports some rich text editing, and one of the places where our logic has to be different from the textinput provided by RN is the text setting logic:

public void maybeSetText(ReactTextUpdate reactTextUpdate) {

However, some of the important members are private and our subclass cannot access them (we work around this now with reflection). It would be nice if we could work with them directly, either using getters and setters or by changing the access. Let me know what you think about this. Thanks.

Changelog

[Android] [Added] - allow custom maybeSetText logic for ReactEditText subclasses

Test Plan

This should work just the same :)

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label May 17, 2019
@react-native-bot react-native-bot added Platform: Android Android applications. Type: Enhancement A new feature or enhancement of an existing feature. labels May 17, 2019
Copy link
Contributor

@cpojer cpojer left a comment

Choose a reason for hiding this comment

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

I think this is fine to share.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

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

@react-native-bot
Copy link
Collaborator

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

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

@react-native-bot react-native-bot added the Merged This PR has been merged. label May 21, 2019
@vonovak vonovak deleted the patch-16 branch May 21, 2019 20:45
facebook-github-bot pushed a commit that referenced this pull request May 23, 2019
…24995)

Summary:
Motivation is the same as in #24927 - when building a custom textinput (eg with rich text editing), one needs custom text processing logic. `ReactTextInputShadowNode` contains https://github.com/facebook/react-native/blob/6671165f69e37a49af8b709b4807f9049f7606c3/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputShadowNode.java#L211

where an instance of `ReactTextUpdate` is created. For the custom use case, we'd like to just change the usage of [`spannedFromShadowNode()`](https://github.com/facebook/react-native/blob/6671165f69e37a49af8b709b4807f9049f7606c3/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputShadowNode.java#L217) to our own implementation.

from there:
It's easy to subclass `ReactTextInputShadowNode` and override `public void onCollectExtraUpdates()` but the problem is that the method accesses private members. It also means overriding more code than necessary as we only care for `spannedFromShadowNode()`.

Solution might be changing the members to protected, but that seemed weird because there are already setters for them. Creating getters also seemed weird, as we'd end up having unused getters hanging around.

So the second way which I find nicer is changing `protected static Spannable spannedFromShadowNode(ReactBaseTextShadowNode textShadowNode, String text)` to just `protected` since that will allow subclasses to override just this behavior.

## Changelog

[Android] [Added] - allow custom spannedFromShadowNode in ReactTextInputShadowNode subclasses
Pull Request resolved: #24995

Differential Revision: D15468066

Pulled By: cpojer

fbshipit-source-id: 73d5f0b9e06f3e02a03bf9db5effac62cecc80c4
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
…k#24927)

Summary:
We're working on a custom EditText that supports some rich text editing, and one of the places where our logic has to be different from the textinput provided by RN is the text setting logic:

https://github.com/facebook/react-native/blob/7abfd23b90db08b426c3c91b0cb6d01d161a9b9e/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactEditText.java#L377

However, some of the important members are private and our subclass cannot access them (we work around this now with reflection). It would be nice if we could work with them directly, either using getters and setters or by changing the  access. Let me know what you think about this. Thanks.

## Changelog

[Android] [Added] - allow custom maybeSetText logic for ReactEditText subclasses
Pull Request resolved: facebook#24927

Differential Revision: D15431682

Pulled By: cpojer

fbshipit-source-id: 91860cadac0798a101ff7df6f6b868f3980ba9b1
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
…acebook#24995)

Summary:
Motivation is the same as in facebook#24927 - when building a custom textinput (eg with rich text editing), one needs custom text processing logic. `ReactTextInputShadowNode` contains https://github.com/facebook/react-native/blob/6671165f69e37a49af8b709b4807f9049f7606c3/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputShadowNode.java#L211

where an instance of `ReactTextUpdate` is created. For the custom use case, we'd like to just change the usage of [`spannedFromShadowNode()`](https://github.com/facebook/react-native/blob/6671165f69e37a49af8b709b4807f9049f7606c3/ReactAndroid/src/main/java/com/facebook/react/views/textinput/ReactTextInputShadowNode.java#L217) to our own implementation.

from there:
It's easy to subclass `ReactTextInputShadowNode` and override `public void onCollectExtraUpdates()` but the problem is that the method accesses private members. It also means overriding more code than necessary as we only care for `spannedFromShadowNode()`.

Solution might be changing the members to protected, but that seemed weird because there are already setters for them. Creating getters also seemed weird, as we'd end up having unused getters hanging around.

So the second way which I find nicer is changing `protected static Spannable spannedFromShadowNode(ReactBaseTextShadowNode textShadowNode, String text)` to just `protected` since that will allow subclasses to override just this behavior.

## Changelog

[Android] [Added] - allow custom spannedFromShadowNode in ReactTextInputShadowNode subclasses
Pull Request resolved: facebook#24995

Differential Revision: D15468066

Pulled By: cpojer

fbshipit-source-id: 73d5f0b9e06f3e02a03bf9db5effac62cecc80c4
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants