-
Notifications
You must be signed in to change notification settings - Fork 24.8k
Fixed - Text with adjustsFontSizeToFit changes the text layout infinitely #33135
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
Conversation
…on Android (facebook#31538)" This reverts commit 5902152.
Hi @alfonsocj! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Yeah, as the author of the original fix (being reverted here), I believe we should back it out because it introduces way more problems than solves. |
@lunaleaps hey, hey! if you are still managing the release process, I believe it should be added to a patch version. 🙇 |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Hey @shergin! Glad to hear from you! Sounds good, I'll put it as a comment on our releases discussion group. It's where we generally make cherry-picks for stable/pre-release now! |
@lunaleaps has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Thanks @lunaleaps! Can we also try to include this fix in the discussion for 0.66.5? I'm not sure if you fix previous versions as well. |
This pull request was successfully merged by Alfonso Curbelo in c1db41f. When will my fix make it into a release? | Upcoming Releases |
Generally no, we usually only target the current stable or release candidate. |
…tely (#33135) Summary: Fixes the infinite loop explained in the issue #33129 by reverting commit 5902152. PR #31538. `onCollectExtraUpdates` is part of the node update cycle. By marking the node as updated `markUpdated()` in `onCollectExtraUpdates` we are restarting the update infinitely. Unfortunately, reverting this PR also reintroduces the original issue #30717 which IMO is minor compared to the infinite loop. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [Android] [Fixed] - Text with adjustsFontSizeToFit changes the text layout infinitely Pull Request resolved: #33135 Test Plan: I added a console.log to the Text `onTextLayout` in [packages/rn-tester/js/examples/Text/TextAdjustsDynamicLayoutExample.js ](https://github.com/facebook/react-native/blob/main/packages/rn-tester/js/examples/Text/TextAdjustsDynamicLayoutExample.js) to see if the infinite loop is gone.   ``` Reviewed By: ShikaSD Differential Revision: D34310218 Pulled By: lunaleaps fbshipit-source-id: 0d40f49d15c562ec25983145897bd95dc182f897
…tely (#33135) Summary: Fixes the infinite loop explained in the issue #33129 by reverting commit 5902152. PR #31538. `onCollectExtraUpdates` is part of the node update cycle. By marking the node as updated `markUpdated()` in `onCollectExtraUpdates` we are restarting the update infinitely. Unfortunately, reverting this PR also reintroduces the original issue #30717 which IMO is minor compared to the infinite loop. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [Android] [Fixed] - Text with adjustsFontSizeToFit changes the text layout infinitely Pull Request resolved: #33135 Test Plan: I added a console.log to the Text `onTextLayout` in [packages/rn-tester/js/examples/Text/TextAdjustsDynamicLayoutExample.js ](https://github.com/facebook/react-native/blob/main/packages/rn-tester/js/examples/Text/TextAdjustsDynamicLayoutExample.js) to see if the infinite loop is gone.   ``` Reviewed By: ShikaSD Differential Revision: D34310218 Pulled By: lunaleaps fbshipit-source-id: 0d40f49d15c562ec25983145897bd95dc182f897
Is there a way to apply this fix without upgrading react-native? I tried patching react-native package, but that doesn't seem to do anything, as if module remains unchanged. |
…tely (facebook#33135) Summary: Fixes the infinite loop explained in the issue facebook#33129 by reverting commit 5902152. PR facebook#31538. `onCollectExtraUpdates` is part of the node update cycle. By marking the node as updated `markUpdated()` in `onCollectExtraUpdates` we are restarting the update infinitely. Unfortunately, reverting this PR also reintroduces the original issue facebook#30717 which IMO is minor compared to the infinite loop. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [Android] [Fixed] - Text with adjustsFontSizeToFit changes the text layout infinitely Pull Request resolved: facebook#33135 Test Plan: I added a console.log to the Text `onTextLayout` in [packages/rn-tester/js/examples/Text/TextAdjustsDynamicLayoutExample.js ](https://github.com/facebook/react-native/blob/main/packages/rn-tester/js/examples/Text/TextAdjustsDynamicLayoutExample.js) to see if the infinite loop is gone.   ``` Reviewed By: ShikaSD Differential Revision: D34310218 Pulled By: lunaleaps fbshipit-source-id: 0d40f49d15c562ec25983145897bd95dc182f897
Summary
Fixes the infinite loop explained in the issue #33129 by reverting commit 5902152. PR #31538.
onCollectExtraUpdates
is part of the node update cycle. By marking the node as updatedmarkUpdated()
inonCollectExtraUpdates
we are restarting the update infinitely.Unfortunately, reverting this PR also reintroduces the original issue #30717 which IMO is minor compared to the infinite loop.
Changelog
[Android] [Fixed] - Text with adjustsFontSizeToFit changes the text layout infinitely
Test Plan
I added a console.log to the Text
onTextLayout
in packages/rn-tester/js/examples/Text/TextAdjustsDynamicLayoutExample.js to see if the infinite loop is gone.