setNativeProps does not work with text property on Text component
#22855
Comments
patch-package
--- a/node_modules/react-native/Libraries/Text/BaseText/RCTBaseTextShadowView.m
+++ b/node_modules/react-native/Libraries/Text/BaseText/RCTBaseTextShadowView.m
@@ -73,6 +73,33 @@ static void RCTInlineViewYogaNodeDirtied(YGNodeRef node)
[super removeReactSubview:subview];
}
+- (NSString *)text
+{
+ NSString *text;
+ for (RCTShadowView *shadowView in self.reactSubviews) {
+ // Special Case: RCTRawTextShadowView
+ if ([shadowView isKindOfClass:[RCTRawTextShadowView class]]) {
+ RCTRawTextShadowView *rawTextShadowView = (RCTRawTextShadowView *)shadowView;
+ text = rawTextShadowView.text;
+ continue;
+ }
+ }
+ return text;
+}
+
+- (void)setText:(NSString *)text
+{
+ for (RCTShadowView *shadowView in self.reactSubviews) {
+ // Special Case: RCTRawTextShadowView
+ if ([shadowView isKindOfClass:[RCTRawTextShadowView class]]) {
+ RCTRawTextShadowView *rawTextShadowView = (RCTRawTextShadowView *)shadowView;
+ rawTextShadowView.text = text;
+ continue;
+ }
+ }
+ [self dirtyLayout];
+}
+
#pragma mark - attributedString
- (NSAttributedString *)attributedTextWithBaseTextAttributes:(nullable RCTTextAttributes *)baseTextAttributes
--- a/node_modules/react-native/Libraries/Text/BaseText/RCTBaseTextViewManager.m
+++ b/node_modules/react-native/Libraries/Text/BaseText/RCTBaseTextViewManager.m
@@ -26,6 +26,7 @@ RCT_EXPORT_MODULE(RCTBaseText)
#pragma mark - Text Attributes
// Color
+RCT_EXPORT_SHADOW_PROPERTY(text, NSString)
RCT_REMAP_SHADOW_PROPERTY(color, textAttributes.foregroundColor, UIColor)
RCT_REMAP_SHADOW_PROPERTY(backgroundColor, textAttributes.backgroundColor, UIColor)
RCT_REMAP_SHADOW_PROPERTY(opacity, textAttributes.opacity, CGFloat)
--- a/node_modules/react-native/Libraries/Text/Text.js
+++ b/node_modules/react-native/Libraries/Text/Text.js
@@ -65,6 +65,7 @@ const viewConfig = {
minimumFontScale: true,
textBreakStrategy: true,
onTextLayout: true,
+ text: true
},
directEventTypes: {
topTextLayout: {makes it works on iOS, provided the props is send in form of string, eg: export default class App extends Component<Props, State> {
textRef = createRef();
state = {
progress: new Animated.Value(0)
};
componentDidMount() {
this.state.progress.addListener(ev => {
this.textRef.current &&
this.textRef.current.setNativeProps({
text: Math.round(ev.value).toString()
});
});
Animated.timing(this.state.progress, {
toValue: 100,
duration: 2000
}).start();
}
render() {
return (
<Animated.View style={styles.container}>
<Text style={styles.welcome} ref={this.textRef}>
-
</Text>
</Animated.View>
);
}
}
If I forget the
Questions:
Next, Android :) |
|
For Android this seems to work: /**
* Copyright (c) 2015-present, Facebook, Inc.
* <p>
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
package com.facebook.react.views.text;
public class ReactTextShadowNode extends ReactBaseTextShadowNode {
// .....
@ReactProp(name = "text")
public void setText(@Nullable String text) {
List<ReactShadowNode> childrens = this.getChildrenList();
if (childrens != null) {
for (ReactShadowNode child : childrens) {
if (child instanceof ReactRawTextShadowNode) {
((ReactRawTextShadowNode) child).setText(text);
}
}
}
markUpdated();
}
}
|
|
If the approach is OK, let me know, I will create a PR for further discussion. |
|
@tychota Thank you for working on this! 👍 Your approach is okay, but we can improve this a bit more.
So, the output of def should be "abcdef". Actually, we probably want to make a case where both the property and nested content are not empty illegal. Adam put a lot of consideration into this topic, so I defer this decision to him. cc @rigdern And maybe we want to go with "value" instead of "text". Adam, IIRC we discussed this as well, what do you think? |
|
The problem I worked on is different but close enough that I have an opinion here. I would say that it's not a bug that I just wanted to clarify that I consider our discussion here to be about a new feature rather than a bug fix. Adding this new feature will introduce some complexity to the
@tychota can you describe the real world scenario you are trying to solve and why passing the text as children to the API FeedbackIf we decide to go forward with this feature, here are some thoughts on the API:
For Adam Comella |
Real Word scenario
Fair point. We actually discussed it with @shergin but offline.
Two examples in my current app:
In both case the performance is slugish. To give you an example with code: Progress (example 2)1. Unoptimized versionclick to see the code ...export class CircleProgression extends PureComponent<IProps, IState> {
public state: IState = {
percentage: 0,
};
private iteration = 0;
private circleDimensions = {
diameter: 190,
progressBarWidth: 15,
get radius() {
return this.diameter / 2 - this.progressBarWidth / 2;
},
};
public componentDidMount() {
setTimeout(this.animateProgress, 300);
}
public render() {
const { chapter } = this.props;
const { percentage } = this.state;
const { diameter, radius, progressBarWidth } = this.circleDimensions;
const perimeter = Math.PI * 2 * radius;
const offset = perimeter * (1 - percentage / 100);
return (
<Container>
<CircleContainer>
<ProgressionCircleBackground>
<Svg height={diameter} width={diameter}>
<Circle
cx={diameter / 2}
cy={diameter / 2}
r={radius}
fill="none"
stroke="#ffffff"
strokeWidth={progressBarWidth}
strokeDasharray={[perimeter]}
strokeDashoffset={offset}
/>
</Svg>
</ProgressionCircleBackground>
<Medallion
source={this.getMedallionSource(chapter)}
resizeMode="contain"
/>
</CircleContainer>
<ProgressionTextContainer>
<ProgressionNumber>{Math.floor(percentage)}</ProgressionNumber>
<ProgressionSymbol>%</ProgressionSymbol>
</ProgressionTextContainer>
</Container>
);
}
private getMedallionSource = (chapter: TChapterName) => {
return chapterData[chapter].imageSrc;
};
private animateProgress = () => {
const { percentage } = this.state;
const maxPercentage = chapterData[this.props.chapter].progressionPercentage;
if (percentage >= maxPercentage) return;
const newProgress = easeInOutExpo(
this.iteration,
0,
maxPercentage / 100,
70
);
this.setState({ percentage: newProgress * 100 });
this.iteration += 1;
requestAnimationFrame(this.animateProgress);
};
}The perf was horrible (drop around 15 fps in debug) still super slugish on release, even on good device (iphoneX and pixel). 2. Animated svgclick to see the code ...export class CircleProgression extends PureComponent<IProps, IState> {
public state: IState = {
percentage: 0,
animatedPercentage: new Animated.Value(INITIAL_PERCENTAGE),
};
public componentDidMount() {
this.listenerId = this.state.animatedPercentage.addListener(e => {
this.setState({percentage: e.value});
});
this.startAnimation();
}
public get maxPercentage() {
return chapterData[this.props.chapter].progressionPercentage;
}
public componentWillUnmount() {
this.state.animatedPercentage.removeListener(this.listenerId);
}
public render() {
const { chapter } = this.props;
const { diameter, radius, progressBarWidth } = this.circleDimensions;
const perimeter = Math.PI * 2 * radius;
const emptyArcSize = perimeter * (1 - this.maxPercentage / 100);
const animatedOffset = this.state.animatedPercentage.interpolate({
inputRange: [INITIAL_PERCENTAGE, this.maxPercentage],
outputRange: [perimeter, emptyArcSize],
});
return (
<Container>
<CircleContainer>
<ProgressionCircleBackground>
<Svg height={diameter} width={diameter}>
<AnimatedCircle
cx={diameter / 2}
cy={diameter / 2}
r={radius}
fill="none"
stroke="#ffffff"
strokeWidth={progressBarWidth}
strokeDasharray={[perimeter]}
strokeDashoffset={animatedOffset}
/>
</Svg>
</ProgressionCircleBackground>
<Medallion
source={this.getMedallionSource(chapter)}
resizeMode="contain"
/>
</CircleContainer>
<ProgressionTextContainer>
<ProgressionNumber>{Math.floor(this.state.percentage)}</ProgressionNumber>
<ProgressionSymbol>%</ProgressionSymbol>
</ProgressionTextContainer>
</Container>
);
}
private startAnimation = () => {
Animated.timing(this.state.animatedPercentage, {
toValue: this.maxPercentage,
delay: ANIMATION_DELAY,
duration: ANIMATION_DURATION,
useNativeDriver: true,
}).start();
};
private getMedallionSource = (chapter: TChapterName) => {
return chapterData[chapter].imageSrc;
};
}We upgraded 3. Trick, using Textinputclick to see the code ...export class CircleProgression extends PureComponent<IProps, IState> {
public state: IState = {
animatedPercentage: new Animated.Value(INITIAL_PERCENTAGE),
};
private progression: React.RefObject<TextInput> = React.createRef();
public componentDidMount() {
this.listenerId = this.state.animatedPercentage.addListener(e => {
this.updateNativePercentage(e.value);
});
this.startAnimation();
}
public get maxPercentage() {
return chapterData[this.props.chapter].progressionPercentage;
}
public componentWillUnmount() {
this.state.animatedPercentage.removeListener(this.listenerId);
}
public render() {
const { chapter } = this.props;
const { diameter, radius, progressBarWidth } = this.circleDimensions;
const perimeter = Math.PI * 2 * radius;
const emptyArcSize = perimeter * (1 - this.maxPercentage / 100);
const animatedOffset = this.state.animatedPercentage.interpolate({
inputRange: [INITIAL_PERCENTAGE, this.maxPercentage],
outputRange: [perimeter, emptyArcSize],
});
return (
<Container>
<CircleContainer>
<ProgressionCircleBackground>
<Svg height={diameter} width={diameter}>
<AnimatedCircle
cx={diameter / 2}
cy={diameter / 2}
r={radius}
fill="none"
stroke="#ffffff"
strokeWidth={progressBarWidth}
strokeDasharray={[perimeter]}
strokeDashoffset={animatedOffset}
/>
</Svg>
</ProgressionCircleBackground>
<Medallion
source={this.getMedallionSource(chapter)}
resizeMode="contain"
/>
</CircleContainer>
<ProgressionNumber ref={this.progression} />
</Container>
);
}
private startAnimation = () => {
Animated.timing(this.state.animatedPercentage, {
toValue: this.maxPercentage,
delay: ANIMATION_DELAY,
duration: ANIMATION_DURATION,
useNativeDriver: true,
}).start();
};
private updateNativePercentage = (newValue: number) => {
const currentProgression = this.progression.current;
if (currentProgression) {
currentProgression.setNativeProps({
text: `${Math.floor(newValue)}%`,
});
}
};
private getMedallionSource = (chapter: TChapterName) => {
return chapterData[chapter].imageSrc;
};
}
const ProgressionNumber = styled.TextInput.attrs({
editable: false,
defaultValue: `${INITIAL_PERCENTAGE}%`,
})`
font-family: ${({ theme }) => theme.fonts.primary.bold};
color: ${({ theme }) => theme.colors.white};
font-size: ${({ theme }) => theme.fontSizes.hugeTitle};
`;Consistant frame rate on JS and native thread, max drop to 53fps, super fluid even on low device (galaxy s3). Slider (example 1)The slider is even worth, user side of view. Since user do a gesture, animation as not only to be fluid but be also in sync. That is impossible to do without nativeDriver (or @kmagiera reanimated). You have an example here : https://blog.bam.tech/developper-news/create-vertical-slider-with-react-native-panresponder While the animation is cool it does not feel native. Is the example clearer, @rigdern ? |
Solutions comparisonSolution 1: props
|
|
@tychota thanks for your detailed posts. Understanding the real world problem you are encountering is very helpful. Reading thru your two examples, I think you are talking about several different problems:
For example in "Progress (example 2) -- 2. Animated svg", you mentioned that upgrading I think this GitHub issue should focus on (1) and (2). Let's talk about what an ideal API might look like. Once that's decided, we can see how to implement it. If your problem is all about animating the value of (assume that Potential Problem: Are Limitation: You can only pass a string to the @tychota Do you see any problems with the above 2 APIs for solving your problems (let's ignore the implementation details of these APIs for now)? Am I correct in assuming that ideally you want to update the value of the Potential App-Level Workaround@tychota Here's a workaround that might work for you today. The idea is that the I haven't tested this so I don't know if it's fast enough for your scenario. Note that I modeled Thoughts on @tychota's SolutionsSolution 1: props
|
I see. The prop is called I wonder if this causes problems if you try to pass an |
Sorry not putting that earlier on.
I think I have a slightly different classification of the problem. By order of importance User problems:
Dev experience:
Those UX problems and DX problems translate into a few technical problems:
Thus we should focus on (1) (2) and (4), in my opinion.
I think it works (for me children is only syntactic sugar for children props) but let me test. If it does not, we should make it works. I will try in my Minimal working example.
I think this works: <Text> It is me, <Text style={boldStyle}>Mario</Text></Text>was it what you meant ?
Sadly it is not enough in both the animated Text (slugish in bad android) and the pan responder slider (not feeling native, even in iPhone X)
Solution 2: seems good concern so I dislike solution 2 a bit more. Plus it require another api to update a props. Solution 3: not sure but I can easily (or not) patch the minimal working example here (https://github.com/tychota/bugSetNativePropsText) and see. I will try later this week and report here. Solution 4: reanimated, and react-native-gesture-handler |
I agree with focusing on (1) and (2) in this issue because they are both
Great! I look forward to hearing about the result of your experiment.
No. I was trying to convey two different API options for using |
|
Hello there 👋 this issue seems to have stalled [after some good back and forth!]. I’ll label it as needing Follow Up, meaning that it will get closed in the future after further stagnation. |
|
Thanks for pinging @alloy, completly forgotten this. You could have closen "stale" it but you chose to ping and that is motivating me to finish that. Thank you :) SummaryTo sum stuff up:
I should test react native Text update for solution 3 but i guess it depends on solution 1 and also #18187 (one year old, @alloy can you ping there tooo so it get merged). Maybe someone on rn team have more insight on animated api and how it works and can answer or explain me. Next steps
|
|
This feature would be awesome!! 🥇 |
|
Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions. |
stale
bot
added
the
Stale
|
Using react-native-reanimated to animate text doesn't work @ android. |
stale
bot
removed
the
Stale
|
Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions. |
stale
bot
added
the
Stale
|
Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information. |
Environment
Description
setNativePropsdoes not work withtextproperty onTextcomponent.Reproducible Demo
See this repo:
https://github.com/tychota/bugSetNativePropsText
Expected:
Real life:
People to CC
Cause hypothesis
AFAIK:
RCTTextand aRCTRawTextRCTText(eg opacity, color, ect), the text itself is handled byRCTRawTextRCTText(or its parentRCTBaseText) to set textNext steps
I'm willing to contribute.
I think we can do something similar to 2307ea6.
Not sure exactly how to get the text since the information must be in attributed string and I would like to keep having one single source of True (hence not duplicating text and adding it as property like previous commit).
The text was updated successfully, but these errors were encountered: