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.

Sign up for GitHub

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

Jump to bottom

fix: apply font size to ReactTextView to fix ellipsis cut #37248

Closed
wants to merge 4 commits into from
Closed

fix: apply font size to ReactTextView to fix ellipsis cut #37248

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e6fc3a
fix: apply font size to ReactTextView to fix ellipsis cut
BeeMargarida Apr 25, 2023
1bbd5af
fix: typo
BeeMargarida Apr 27, 2023
9c7d14d
feat: improve comment and add example of the problem to rn-tester
BeeMargarida May 4, 2023
d0c8252
fix: add similar ellipsis fix for letter spacing
BeeMargarida May 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .../ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextAnchorViewManager.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ public void setAdjustFontSizeToFit(ReactTextView view, boolean adjustsFontSizeTo
view.setAdjustFontSizeToFit(adjustsFontSizeToFit);
}

@ReactProp(name = ViewProps.FONT_SIZE)
public void setFontSize(ReactTextView view, float fontSize) {
view.setFontSize(fontSize);
}

@ReactProp(name = ViewProps.LETTER_SPACING, defaultFloat = Float.NaN)
public void setLetterSpacing(ReactTextView view, float letterSpacing) {
view.setLetterSpacing(letterSpacing);
}

@ReactProp(name = ViewProps.TEXT_ALIGN_VERTICAL)
public void setTextAlignVertical(ReactTextView view, @Nullable String textAlignVertical) {
if (textAlignVertical == null || "auto".equals(textAlignVertical)) {
Expand Down
39 changes: 39 additions & 0 deletions .../react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import android.text.TextUtils;
import android.text.method.LinkMovementMethod;
import android.text.util.Linkify;
import android.util.TypedValue;
import android.view.Gravity;
import android.view.MotionEvent;
import android.view.View;
Expand Down Expand Up @@ -57,6 +58,8 @@ public class ReactTextView extends AppCompatTextView implements ReactCompoundVie
private int mNumberOfLines;
private TextUtils.TruncateAt mEllipsizeLocation;
private boolean mAdjustsFontSizeToFit;
private float mFontSize = Float.NaN;
private float mLetterSpacing = Float.NaN;
private int mLinkifyMaskType;
private boolean mNotifyOnInlineViewLayout;
private boolean mTextIsSelectable;
Expand Down Expand Up @@ -582,6 +585,29 @@ public void setAdjustFontSizeToFit(boolean adjustsFontSizeToFit) {
mAdjustsFontSizeToFit = adjustsFontSizeToFit;
}

public void setFontSize(float fontSize) {
mFontSize =
mAdjustsFontSizeToFit
? (float) Math.ceil(PixelUtil.toPixelFromSP(fontSize))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we using SP here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I ended up basing this code in the snippet from TextAttributes.java.

Should it be removed?

Copy link
Contributor

@NickGerleman NickGerleman May 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It looks like the code there is using it in conjunction with allowFontScaling to find font size. Not anything with adjustFontSizeToFit.

Can we reuse getEffectiveFontSize()? E.g. have the view manager or ShadowNode call setTextSize()?

Copy link
Contributor Author

@BeeMargarida BeeMargarida May 11, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, you're right!

Can we reuse getEffectiveFontSize()?

Not directly because ReactTextView does not use TextAttributes. We could extract that logic and use a static method, but it would require passing mMaxFontSizeMultiplier and mAllowFontScaling (or just use defaults), which I'm not sure it would be the best, I'm not really sure what's the implications of this.

We could just remove the adjust logic and just use toPixelFromDIP, would it be best?

: (float) Math.ceil(PixelUtil.toPixelFromDIP(fontSize));

applyTextAttributes();
}

public void setLetterSpacing(float letterSpacing) {
if (Float.isNaN(letterSpacing)) {
return;
}

float letterSpacingPixels = PixelUtil.toPixelFromDIP(letterSpacing);

// `letterSpacingPixels` and `getEffectiveFontSize` are both in pixels,
// yielding an accurate em value.
mLetterSpacing = letterSpacingPixels / mFontSize;

applyTextAttributes();
}

public void setEllipsizeLocation(TextUtils.TruncateAt ellipsizeLocation) {
mEllipsizeLocation = ellipsizeLocation;
}
Expand Down Expand Up @@ -651,4 +677,17 @@ protected boolean dispatchHoverEvent(MotionEvent event) {

return super.dispatchHoverEvent(event);
}

private void applyTextAttributes() {
// Workaround for an issue where text can be cut off with an ellipsis when
// using certain font sizes and padding. Sets the provided text size and
// letter spacing to ensure consistent rendering and prevent cut-off.
if (!Float.isNaN(mFontSize)) {
setTextSize(TypedValue.COMPLEX_UNIT_PX, mFontSize);
}

if (!Float.isNaN(mLetterSpacing)) {
super.setLetterSpacing(mLetterSpacing);
}
}
}
4 changes: 4 additions & 0 deletions packages/rn-tester/js/examples/Text/TextExample.android.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,10 @@ class TextExample extends React.Component<{...}> {
Maximum of one line no matter now much I write here. If I keep
writing it{"'"}ll just truncate after one line
</Text>
<Text style={{fontSize: 31}} numberOfLines={1}>
Maximum of one line no matter now much I write here. If I keep
writing it{"'"}ll just truncate after one line
</Text>
<Text numberOfLines={2} style={{marginTop: 20}}>
Maximum of two lines no matter now much I write here. If I keep
writing it{"'"}ll just truncate after two lines
Expand Down