-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[ASDisplayNode] Fix an issue that causes a node to sometimes return an outdated calculated size or size range #808
[ASDisplayNode] Fix an issue that causes a node to sometimes return an outdated calculated size or size range #808
Conversation
…n outdated calculated size or size range
eda7613
to
d509970
Compare
@@ -305,24 +305,22 @@ - (void)_u_measureNodeWithBoundsIfNecessary:(CGRect)bounds | |||
} | |||
|
|||
CGSize boundsSizeForLayout = ASCeilSizeValues(bounds.size); | |||
NSUInteger calculatedVersion = _calculatedDisplayNodeLayout->version; |
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.
Note to reviewers: No change in the logic of this method (-_u_measureNodeWithBoundsIfNecessary:
), just call the new isValid()
function to (hopefully) increase readability.
30b1c9d
to
a406cef
Compare
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.
Nice! Also great to increase our test base.
@nguyenhuy any updates on this? |
@nguyenhuy should we be moving forward on this? I'm considering (finally) pulling in the _layoutVersion changes, which I avoided for a while...wondering if this is an important fix before doing so. cc @maicki, @Adlai-Holler |
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.
Looks good! I should have had this logic from the beginning.
We should never return any layout whose version is out of date, period
Wooohoo 🙌 🎉 ! I depended heavily on this feature working. Thank you so much guys 😄 |
@Adlai-Holler Thanks for syncing with master! |
When asked for the calculated size or constrained size for the calculated layout, prefer the pending layout only if its version is up-to-date. This should fix the issue reported by @codeOfRobin (here) and reproducible by this sample project.