-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
Fix relayout of inline views #21968
Closed
Closed
Fix relayout of inline views #21968
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If a view inside of an inline view became dirty (e.g. its top/left prop changed), its position would not update on screen. This is because Yoga didn't know the view needed to be relaid out because Yoga's dirty signal didn't propagate all the way up to the root. The problem is that inline views don't have a parent in the Yoga tree causing Yoga's dirtiness signal propagation to get cut off early. The fix is, when an inline views gets dirty, mark the parent Text's Yoga node as dirty. This will cause Yoga's dirtiness signal to propagate all the way up to the root node. Yoga has a hook to inform you when your node is marked as dirty: `YGNodeSetDirtiedFunc`. We leverage this to find out when an inline view's Yoga node gets dirtied. React Native almost handled this case. Everything worked fine as long as the inline view was nested inside of a virtual text node like this: ``` <Text> <Text> <InlineView /> </Text> </Text> ``` However, the bug repros when the inline view is nested in a non-virtual text node: ``` <Text> <InlineView /> </Text> ``` The fix is to move the special dirtiness propagation logic from `RCTVirtualTextShadowView` to `RCTBaseTextShadowView`. **Test Plan** Created an inline view. Tested the following kinds of updates on the inline view's content: - Moved the content - Removed the content - Added the content Tested this for an inline view that is directly inside of a text node as well as one that is nested under a virtual text node. Here's the code I used for the inline view that moved its content after 2 seconds: ``` const RN = require('react-native'); const React = require('react'); export default class InlineView extends React.Component { constructor(props, context) { super(props, context); this.state = { posBottom: false }; } componentDidMount() { super.componentDidMount && super.componentDidMount(); setTimeout(() => { this.setState({ posBottom: true }); }, 2000); } render() { const pos = this.state.posBottom ? 25 : 0; const color = this.state.posBottom ? 'pink' : 'green'; return ( <RN.View style={{ width: 50, height: 50, backgroundColor: 'steelblue'}}> <RN.View style={{ width: 25, height: 25, top: pos, left: pos, backgroundColor: color }} /> </RN.View> ); } } ``` Adam Comella Microsoft Corp.
Generated by 🚫 dangerJS |
facebook-github-bot
approved these changes
Oct 31, 2018
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@shergin is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
kelset
pushed a commit
that referenced
this pull request
Nov 9, 2018
Summary: If a view inside of an inline view became dirty (e.g. its top/left prop changed), its position would not update on screen. This is because Yoga didn't know the view needed to be relaid out because Yoga's dirty signal didn't propagate all the way up to the root. The problem is that inline views don't have a parent in the Yoga tree causing Yoga's dirtiness signal propagation to get cut off early. The fix is, when an inline views gets dirty, mark the parent Text's Yoga node as dirty. This will cause Yoga's dirtiness signal to propagate all the way up to the root node. Yoga has a hook to inform you when your node is marked as dirty: `YGNodeSetDirtiedFunc`. We leverage this to find out when an inline view's Yoga node gets dirtied. React Native almost handled this case. Everything worked fine as long as the inline view was nested inside of a virtual text node like this: ``` <Text> <Text> <InlineView /> </Text> </Text> ``` However, the bug repros when the inline view is nested in a non-virtual text node: ``` <Text> <InlineView /> </Text> ``` The fix is to move the special dirtiness propagation logic from `RCTVirtualTextShadowView` to `RCTBaseTextShadowView`. **Test Plan** Created an inline view. Tested the following kinds of updates on the inline view's content: - Moved the content - Removed the content - Added the content Tested this for an inline view that is directly inside of a text node as well as one that is nested under a virtual text node. Here's the code I used for the inline view that moved its content after 2 seconds: ``` const RN = require('react-native'); const React = require('react'); export default class InlineView extends React.Component { constructor(props, context) { super(props, context); this.state = { posBottom: false }; } componentDidMount() { super.componentDidMount && super.componentDidMount(); setTimeout(() => { this.setState({ posBottom: true }); }, 2000); } render() { const pos = this.state.posBottom ? 25 : 0; const color = this.state.posBottom ? 'pink' : 'green'; return ( <RN.View style={{ width: 50, height: 50, backgroundColor: 'steelblue'}}> <RN.View style={{ width: 25, height: 25, top: pos, left: pos, backgroundColor: color }} /> </RN.View> ); } } ``` **Release Notes** [IOS] [BUGFIX] [Text] - Fix case where content of inline views didn't get relaid out Adam Comella Microsoft Corp. Pull Request resolved: #21968 Differential Revision: D12873795 Pulled By: shergin fbshipit-source-id: bbc9f5d3ef25063b0015cec8c4aaf2e41ecd60a8
t-nanava
pushed a commit
to microsoft/react-native-macos
that referenced
this pull request
Jun 17, 2019
Summary: If a view inside of an inline view became dirty (e.g. its top/left prop changed), its position would not update on screen. This is because Yoga didn't know the view needed to be relaid out because Yoga's dirty signal didn't propagate all the way up to the root. The problem is that inline views don't have a parent in the Yoga tree causing Yoga's dirtiness signal propagation to get cut off early. The fix is, when an inline views gets dirty, mark the parent Text's Yoga node as dirty. This will cause Yoga's dirtiness signal to propagate all the way up to the root node. Yoga has a hook to inform you when your node is marked as dirty: `YGNodeSetDirtiedFunc`. We leverage this to find out when an inline view's Yoga node gets dirtied. React Native almost handled this case. Everything worked fine as long as the inline view was nested inside of a virtual text node like this: ``` <Text> <Text> <InlineView /> </Text> </Text> ``` However, the bug repros when the inline view is nested in a non-virtual text node: ``` <Text> <InlineView /> </Text> ``` The fix is to move the special dirtiness propagation logic from `RCTVirtualTextShadowView` to `RCTBaseTextShadowView`. **Test Plan** Created an inline view. Tested the following kinds of updates on the inline view's content: - Moved the content - Removed the content - Added the content Tested this for an inline view that is directly inside of a text node as well as one that is nested under a virtual text node. Here's the code I used for the inline view that moved its content after 2 seconds: ``` const RN = require('react-native'); const React = require('react'); export default class InlineView extends React.Component { constructor(props, context) { super(props, context); this.state = { posBottom: false }; } componentDidMount() { super.componentDidMount && super.componentDidMount(); setTimeout(() => { this.setState({ posBottom: true }); }, 2000); } render() { const pos = this.state.posBottom ? 25 : 0; const color = this.state.posBottom ? 'pink' : 'green'; return ( <RN.View style={{ width: 50, height: 50, backgroundColor: 'steelblue'}}> <RN.View style={{ width: 25, height: 25, top: pos, left: pos, backgroundColor: color }} /> </RN.View> ); } } ``` **Release Notes** [IOS] [BUGFIX] [Text] - Fix case where content of inline views didn't get relaid out Adam Comella Microsoft Corp. Pull Request resolved: facebook#21968 Differential Revision: D12873795 Pulled By: shergin fbshipit-source-id: bbc9f5d3ef25063b0015cec8c4aaf2e41ecd60a8
aleclarson
pushed a commit
to alloc/react-native-macos
that referenced
this pull request
Mar 5, 2020
Summary: If a view inside of an inline view became dirty (e.g. its top/left prop changed), its position would not update on screen. This is because Yoga didn't know the view needed to be relaid out because Yoga's dirty signal didn't propagate all the way up to the root. The problem is that inline views don't have a parent in the Yoga tree causing Yoga's dirtiness signal propagation to get cut off early. The fix is, when an inline views gets dirty, mark the parent Text's Yoga node as dirty. This will cause Yoga's dirtiness signal to propagate all the way up to the root node. Yoga has a hook to inform you when your node is marked as dirty: `YGNodeSetDirtiedFunc`. We leverage this to find out when an inline view's Yoga node gets dirtied. React Native almost handled this case. Everything worked fine as long as the inline view was nested inside of a virtual text node like this: ``` <Text> <Text> <InlineView /> </Text> </Text> ``` However, the bug repros when the inline view is nested in a non-virtual text node: ``` <Text> <InlineView /> </Text> ``` The fix is to move the special dirtiness propagation logic from `RCTVirtualTextShadowView` to `RCTBaseTextShadowView`. **Test Plan** Created an inline view. Tested the following kinds of updates on the inline view's content: - Moved the content - Removed the content - Added the content Tested this for an inline view that is directly inside of a text node as well as one that is nested under a virtual text node. Here's the code I used for the inline view that moved its content after 2 seconds: ``` const RN = require('react-native'); const React = require('react'); export default class InlineView extends React.Component { constructor(props, context) { super(props, context); this.state = { posBottom: false }; } componentDidMount() { super.componentDidMount && super.componentDidMount(); setTimeout(() => { this.setState({ posBottom: true }); }, 2000); } render() { const pos = this.state.posBottom ? 25 : 0; const color = this.state.posBottom ? 'pink' : 'green'; return ( <RN.View style={{ width: 50, height: 50, backgroundColor: 'steelblue'}}> <RN.View style={{ width: 25, height: 25, top: pos, left: pos, backgroundColor: color }} /> </RN.View> ); } } ``` **Release Notes** [IOS] [BUGFIX] [Text] - Fix case where content of inline views didn't get relaid out Adam Comella Microsoft Corp. Pull Request resolved: facebook/react-native#21968 Differential Revision: D12873795 Pulled By: shergin fbshipit-source-id: bbc9f5d3ef25063b0015cec8c4aaf2e41ecd60a8
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a view inside of an inline view became dirty (e.g. its top/left prop changed), its position would not update on screen. This is because Yoga didn't know the view needed to be relaid out because Yoga's dirty signal didn't propagate all the way up to the root.
The problem is that inline views don't have a parent in the Yoga tree causing Yoga's dirtiness signal propagation to get cut off early. The fix is, when an inline views gets dirty, mark the parent Text's Yoga node as dirty. This will cause Yoga's dirtiness signal to propagate all the way up to the root node.
Yoga has a hook to inform you when your node is marked as dirty:
YGNodeSetDirtiedFunc
. We leverage this to find out when an inline view's Yoga node gets dirtied.React Native almost handled this case. Everything worked fine as long as the inline view was nested inside of a virtual text node like this:
However, the bug repros when the inline view is nested in a non-virtual text node:
The fix is to move the special dirtiness propagation logic from
RCTVirtualTextShadowView
toRCTBaseTextShadowView
.Test Plan
Created an inline view. Tested the following kinds of updates on the inline view's content:
Tested this for an inline view that is directly inside of a text node as well as one that is nested under a virtual text node.
Here's the code I used for the inline view that moved its content after 2 seconds:
Release Notes
[IOS] [BUGFIX] [Text] - Fix case where content of inline views didn't get relaid out
Adam Comella
Microsoft Corp.