fix(ios): force zero width for empty Text component#55472
fix(ios): force zero width for empty Text component#55472Hector-Zhuang wants to merge 1 commit intofacebook:mainfrom
Conversation
| layoutContext:layoutContext | ||
| layoutConstraints:layoutConstraints]; | ||
|
|
||
| if (originalAtributedString.isEmpty()) { |
There was a problem hiding this comment.
Could you leave a quick comment on why we do this
The “right fix” is for TextLayoutManager to use base attributes when the string is empty, instead of placeholder, like Android does, but that’s a good amount of work, so this workaround makes sense to me
There was a problem hiding this comment.
This is to check if originalAtributedString is empty. If it's empty, it should not be displayed, while attributedString has a placeholder "i"
|
Note, this is long time behavior of new architecture. Commit moving where placeholder was added so that Android wasn’t forced to have placeholder |
|
@NickGerleman |
Please see 9fe20d4 for the history here. It previously effected all platforms when the logic to insert I was in ShadowNode. I moved it out to each platform’s ViewManager, but on Android, to provide right height with no text, we need to resolve height of attributedstring without fragments. So the intention, is that text layout managers could measure empty strings using base text attributes of atteibutedstring. Only made the changes for Android though, so logic here for placeholder was to keep previous behavior. |
|
@NickGerleman has imported this pull request. If you are a Meta employee, you can view this in D94011915. |
|
@NickGerleman merged this pull request in 537d2a6. |
|
This pull request was successfully merged by @Hector-Zhuang in 537d2a6 When will my fix make it into a release? | How to file a pick request? |
Summary: This PR fixes a layout regression in iO where empty <Text /> components render with a non-zero width. The Motivation: In commit(facebook@9fe20d4), the logic to add a placeholder "I" to empty strings was moved from ShadowNode to the platform-specific TextLayoutManager. While the "I" placeholder provides the correct height, the measurement result also includes the width of the "I" character. This causes empty components to take up horizontal space. ## Changelog: [IOS] [FIXED] - Force zero width for empty Text components in Fabric to prevent ghost layout artifacts. Pull Request resolved: facebook#55472 Test Plan: Render a component with an empty string and a background color: ``` <Text style={{ backgroundColor: 'red' }} /> ``` Before: A small red vertical sliver (the width of "I") is visible. After: The component is invisible (0 width) ## Related issue: facebook#55468 Reviewed By: cipolleschi Differential Revision: D94011915 Pulled By: NickGerleman fbshipit-source-id: 8408dc43f06bd67e5938004148a4c9ebd3939864
Summary:
This PR fixes a layout regression in iO where empty components render with a non-zero width.
The Motivation: In commit(9fe20d4), the logic to add a placeholder "I" to empty strings was moved from ShadowNode to the platform-specific TextLayoutManager.
While the "I" placeholder provides the correct height, the measurement result also includes the width of the "I" character. This causes empty components to take up horizontal space.
Changelog:
[IOS] [FIXED] - Force zero width for empty Text components in Fabric to prevent ghost layout artifacts.
Test Plan:
Render a component with an empty string and a background color:
Before: A small red vertical sliver (the width of "I") is visible.
After: The component is invisible (0 width)
Related issue:
#55468