Skip to content
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

iOS: Support inline view truncation #21456

Closed
wants to merge 2 commits into from

Conversation

rigdern
Copy link
Contributor

@rigdern rigdern commented Oct 2, 2018

If text is truncated and an inline view appears after the truncation point, the user should not see the inline view. Instead, we have a bug such that the inline view is always visible at the end of the visible text.

This commit fixes this by marking the inline view as hidden if it appears after the truncation point.

This appears to be a regression. React Native used to have logic similar to what this commit is adding:

NSRange truncatedGlyphRange = [layoutManager truncatedGlyphRangeInLineFragmentForGlyphAtIndex:range.location];
BOOL childIsTruncated = NSIntersectionRange(range, truncatedGlyphRange).length != 0;
[child collectUpdatedFrames:viewsWithNewFrame
withFrame:childFrame
hidden:childIsTruncated
absolutePosition:absolutePosition];

Before fix

Inline view (blue square) is visible even though it appears after the truncation point:

image

The full text being rendered was:

<Text numberOfLines={1}>
  Lorem ipsum dolor sit amet, consectetur adipiscing elit,
  sed do eiusmod tempor incididunt ut labore et dolore magna
  <View style={{width: 50, height: 50, backgroundColor: 'steelblue'}} />
</Text>

After fix

Inline view is properly truncated:

image

Test Plan

Tested that the inline view is hidden if it appears after the truncation point when numberOfLines is 1 and 2. Similarly, verified that the inline view is visible if it appears before the truncation point.

Release Notes

[IOS] [BUGFIX] [Text] - Fix case where inline view is visible even though it should have been truncated

Adam Comella
Microsoft Corp.

@rigdern rigdern requested a review from shergin as a code owner October 2, 2018 23:18
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 2, 2018
@react-native-bot react-native-bot added Platform: iOS iOS applications. 🔶Components Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. labels Oct 3, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hramos has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@shergin shergin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nested text is my favorite feature! True! ❤️

Libraries/Text/Text/RCTTextShadowView.m Show resolved Hide resolved
Adam Comella added 2 commits November 15, 2018 13:47
If text is truncated and an inline view appears after the truncation point, the user should not see the inline view. Instead, we have a bug such that the inline view is always visible at the end of the visible text.

This commit fixes this by marking the inline view as hidden if it appears after the truncation point.

**Before fix**

Inline view is visible even though it appears after the truncation point:

![image](https://user-images.githubusercontent.com/199935/46381972-7d39d380-c65d-11e8-8354-91f65af651e4.png)

**After fix**

Inline view is properly truncated:

![image](https://user-images.githubusercontent.com/199935/46381918-3ba92880-c65d-11e8-8e61-1d6a1aae2c72.png)

**Test Plan**

Tested that the inline view is hidden if it appears after the truncation point when `numberOfLines` is 1 and 2. Similarly, verified that the inline view is visible if it appears before the truncation point.

Adam Comella
Microsoft Corp.
@rigdern rigdern force-pushed the rigdern/inlineViewTruncation branch from dd1552d to e8500ef Compare November 15, 2018 22:14
@pull-bot
Copy link

Warnings
⚠️

📋 Release Notes - This PR appears to be missing Release Notes.

Generated by 🚫 dangerJS

@react-native-bot react-native-bot added ✅Release Notes and removed Missing Changelog This PR appears to be missing a changelog, or they are incorrectly formatted. labels Nov 15, 2018
@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Nov 20, 2018
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a 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.

@react-native-bot
Copy link
Collaborator

@rigdern merged commit 70826db into facebook:master.

@facebook facebook locked as resolved and limited conversation to collaborators Nov 20, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Nov 20, 2018
kelset pushed a commit that referenced this pull request Nov 26, 2018
Summary:
If text is truncated and an inline view appears after the truncation point, the user should not see the inline view. Instead, we have a bug such that the inline view is always visible at the end of the visible text.

This commit fixes this by marking the inline view as hidden if it appears after the truncation point.

This appears to be a regression. React Native used to have logic similar to what this commit is adding: https://github.com/facebook/react-native/blob/1e2a924ba60001c6f0587c7561536f00b2922cbf/Libraries/Text/RCTShadowText.m#L186-L192

**Before fix**

Inline view (blue square) is visible even though it appears after the truncation point:

![image](https://user-images.githubusercontent.com/199935/46382038-d3a71200-c65d-11e8-8179-2ce4aad8d010.png)

The full text being rendered was:

```
<Text numberOfLines={1}>
  Lorem ipsum dolor sit amet, consectetur adipiscing elit,
  sed do eiusmod tempor incididunt ut labore et dolore magna
  <View style={{width: 50, height: 50, backgroundColor: 'steelblue'}} />
</Text>
```

**After fix**

Inline view is properly truncated:

![image](https://user-images.githubusercontent.com/199935/46382067-fdf8cf80-c65d-11e8-84ea-e2b71c229dae.png)

**Test Plan**

Tested that the inline view is hidden if it appears after the truncation point when `numberOfLines` is 1 and 2. Similarly, verified that the inline view is visible if it appears before the truncation point.

**Release Notes**

[IOS] [BUGFIX] [Text] - Fix case where inline view is visible even though it should have been truncated

Adam Comella
Microsoft Corp.
Pull Request resolved: #21456

Differential Revision: D10182991

Pulled By: shergin

fbshipit-source-id: a5bddddb1bb8672b61d4feaa04013a92c8224155
@rigdern rigdern deleted the rigdern/inlineViewTruncation branch January 28, 2019 21:30
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
t-nanava pushed a commit to microsoft/react-native-macos that referenced this pull request Jun 17, 2019
Summary:
If text is truncated and an inline view appears after the truncation point, the user should not see the inline view. Instead, we have a bug such that the inline view is always visible at the end of the visible text.

This commit fixes this by marking the inline view as hidden if it appears after the truncation point.

This appears to be a regression. React Native used to have logic similar to what this commit is adding: https://github.com/facebook/react-native/blob/1e2a924ba60001c6f0587c7561536f00b2922cbf/Libraries/Text/RCTShadowText.m#L186-L192

**Before fix**

Inline view (blue square) is visible even though it appears after the truncation point:

![image](https://user-images.githubusercontent.com/199935/46382038-d3a71200-c65d-11e8-8179-2ce4aad8d010.png)

The full text being rendered was:

```
<Text numberOfLines={1}>
  Lorem ipsum dolor sit amet, consectetur adipiscing elit,
  sed do eiusmod tempor incididunt ut labore et dolore magna
  <View style={{width: 50, height: 50, backgroundColor: 'steelblue'}} />
</Text>
```

**After fix**

Inline view is properly truncated:

![image](https://user-images.githubusercontent.com/199935/46382067-fdf8cf80-c65d-11e8-84ea-e2b71c229dae.png)

**Test Plan**

Tested that the inline view is hidden if it appears after the truncation point when `numberOfLines` is 1 and 2. Similarly, verified that the inline view is visible if it appears before the truncation point.

**Release Notes**

[IOS] [BUGFIX] [Text] - Fix case where inline view is visible even though it should have been truncated

Adam Comella
Microsoft Corp.
Pull Request resolved: facebook#21456

Differential Revision: D10182991

Pulled By: shergin

fbshipit-source-id: a5bddddb1bb8672b61d4feaa04013a92c8224155
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. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants