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 overriding spannedFromShadowNode in ReactTextInputShadowNode #24995

Closed

Conversation

vonovak
Copy link
Collaborator

@vonovak vonovak commented May 22, 2019

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

public void onCollectExtraUpdates(UIViewOperationQueue uiViewOperationQueue) {

where an instance of ReactTextUpdate is created. For the custom use case, we'd like to just change the usage of spannedFromShadowNode() 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 a little 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

Test Plan

this will 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 22, 2019
@vonovak vonovak changed the title allow custom spannedFromShadowNode in ReactTextInputShadowNode subcla… overridable spannedFromShadowNode in ReactTextInputShadowNode May 22, 2019
@vonovak vonovak changed the title overridable spannedFromShadowNode in ReactTextInputShadowNode allow overriding spannedFromShadowNode in ReactTextInputShadowNode May 22, 2019
@react-native-bot react-native-bot added Component: TextInput Related to the TextInput component. Platform: Android Android applications. Type: Enhancement A new feature or enhancement of an existing feature. labels May 22, 2019
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.

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.

Sounds good!

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.

Sounds good!

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @vonovak in 04564a0.

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 23, 2019
@vonovak vonovak deleted the allowCustomSpannedFromShadowNode branch June 4, 2019 11:32
facebook-github-bot pushed a commit that referenced this pull request Jun 18, 2019
…wView subclasses (#25283)

Summary:
This is similar to #24995 but for iOS.
Motivation: when building a custom Text or TextInput (eg with rich text support), one needs custom text processing logic (turning the JS text value to a `NSAttributedString`).

RCTBaseTextShadowView contains `- (NSAttributedString *)attributedTextWithBaseTextAttributes:(nullable RCTTextAttributes *)baseTextAttributes;`

https://github.com/facebook/react-native/blob/853c667eb5a66612c33e0786ab6c458dcaee6133/Libraries/Text/BaseText/RCTBaseTextShadowView.m#L78

This method, given `self.reactSubviews` creates and returns an instance of [NSMutableAttributedString](https://developer.apple.com/documentation/foundation/nsmutableattributedstring?language=objc). It's currently possible to override this method in subclasses, but the original implementation reads and writes two cache fields which are private. This PR just changes the access modifiers so that subclasses of `RCTBaseTextShadowView` can also access the caching fields.

## Changelog

Not needed I guess.
Pull Request resolved: #25283

Test Plan: This will work just the same - works locally, CI should pass.

Differential Revision: D15873741

Pulled By: cpojer

fbshipit-source-id: dd26a4241f2ac6c0870b6c302939e2f3b0ecc8ff
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
M-i-k-e-l pushed a commit to M-i-k-e-l/react-native that referenced this pull request Mar 10, 2020
…wView subclasses (facebook#25283)

Summary:
This is similar to facebook#24995 but for iOS.
Motivation: when building a custom Text or TextInput (eg with rich text support), one needs custom text processing logic (turning the JS text value to a `NSAttributedString`).

RCTBaseTextShadowView contains `- (NSAttributedString *)attributedTextWithBaseTextAttributes:(nullable RCTTextAttributes *)baseTextAttributes;`

https://github.com/facebook/react-native/blob/853c667eb5a66612c33e0786ab6c458dcaee6133/Libraries/Text/BaseText/RCTBaseTextShadowView.m#L78

This method, given `self.reactSubviews` creates and returns an instance of [NSMutableAttributedString](https://developer.apple.com/documentation/foundation/nsmutableattributedstring?language=objc). It's currently possible to override this method in subclasses, but the original implementation reads and writes two cache fields which are private. This PR just changes the access modifiers so that subclasses of `RCTBaseTextShadowView` can also access the caching fields.

## Changelog

Not needed I guess.
Pull Request resolved: facebook#25283

Test Plan: This will work just the same - works locally, CI should pass.

Differential Revision: D15873741

Pulled By: cpojer

fbshipit-source-id: dd26a4241f2ac6c0870b6c302939e2f3b0ecc8ff
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. Component: TextInput Related to the TextInput component. 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