Skip to content

Commit

Permalink
fix(Android) drawing additional empty line when 'textAlign' is set to…
Browse files Browse the repository at this point in the history
… 'justify' (#47122)

Summary:
Fixes #46908

The `justificationMode` is not set for multiline text without unicode characters with known width on both architectures. This caused the issue of drawing additional empty line at the end of `TextView` because Yoga thought that text takes 5 lines and falsely calculated it's height.

Currently, on the old architecture, the `justificationMode` is set only on text that is not boring (contains unicode characters) with unknown width. I am not sure why is that, so I am opening this as a draft for now as I am still checking if it doesn't break anything.

## Changelog:

<!-- Help reviewers and the release process by writing your own changelog entry.

Pick one each for the category and type tags:

[ANDROID|GENERAL|IOS|INTERNAL] [BREAKING|ADDED|CHANGED|DEPRECATED|REMOVED|FIXED|SECURITY] - Message

For more details, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[ANDROID] [FIXED] - fix generating empty line at the end of multiline text view when `textAlign` is set to `justify`

Pull Request resolved: #47122

Test Plan: I've tested on both architectures on repro provided in the issue.

Reviewed By: javache

Differential Revision: D65002386

Pulled By: NickGerleman

fbshipit-source-id: 0187956c88e6eb1e637c24e82b3052cc82581a64
  • Loading branch information
coado authored and facebook-github-bot committed Oct 29, 2024
1 parent dacada7 commit 08e8f6a
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 24 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,10 @@ private Layout measureSpannedText(Spannable text, float width, YogaMeasureMode w
.setBreakStrategy(mTextBreakStrategy)
.setHyphenationFrequency(mHyphenationFrequency);

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
builder.setJustificationMode(mJustificationMode);
}

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
builder.setUseLineSpacingFromFallbacks(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,9 @@ protected void onDraw(Canvas canvas) {
getBreakStrategy(),
getHyphenationFrequency(),
// always passing ALIGN_NORMAL here should be fine, since this method doesn't depend on
// how exacly lines are aligned, just their width
// how exactly lines are aligned, just their width
Layout.Alignment.ALIGN_NORMAL,
(Build.VERSION.SDK_INT < Build.VERSION_CODES.O) ? -1 : getJustificationMode(),
getPaint());
setText(getSpanned());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,13 +146,41 @@ public static boolean isRTL(MapBuffer attributedString) {
== LayoutDirection.RTL;
}

private static Layout.Alignment getTextAlignment(MapBuffer attributedString, Spannable spanned) {
@Nullable
private static String getTextAlignmentAttr(MapBuffer attributedString) {
// TODO: Don't read AS_KEY_FRAGMENTS, which may be expensive, and is not present when using
// cached Spannable
if (!attributedString.contains(AS_KEY_FRAGMENTS)) {
return Layout.Alignment.ALIGN_NORMAL;
return null;
}

MapBuffer fragments = attributedString.getMapBuffer(AS_KEY_FRAGMENTS);
if (fragments.getCount() != 0) {
MapBuffer fragment = fragments.getMapBuffer(0);
MapBuffer textAttributes = fragment.getMapBuffer(FR_KEY_TEXT_ATTRIBUTES);

if (textAttributes.contains(TextAttributeProps.TA_KEY_ALIGNMENT)) {
return textAttributes.getString(TextAttributeProps.TA_KEY_ALIGNMENT);
}
}

return null;
}

private static int getTextJustificationMode(@Nullable String alignmentAttr) {
if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) {
return -1;
}

if (alignmentAttr != null && alignmentAttr.equals("justified")) {
return Layout.JUSTIFICATION_MODE_INTER_WORD;
}

return Layout.JUSTIFICATION_MODE_NONE;
}

private static Layout.Alignment getTextAlignment(
MapBuffer attributedString, Spannable spanned, @Nullable String alignmentAttr) {
// Android will align text based on the script, so normal and opposite alignment needs to be
// swapped when the directions of paragraph and script don't match.
// I.e. paragraph is LTR but script is RTL, text needs to be aligned to the left, which means
Expand All @@ -165,23 +193,15 @@ private static Layout.Alignment getTextAlignment(MapBuffer attributedString, Spa
Layout.Alignment alignment =
swapNormalAndOpposite ? Layout.Alignment.ALIGN_OPPOSITE : Layout.Alignment.ALIGN_NORMAL;

MapBuffer fragments = attributedString.getMapBuffer(AS_KEY_FRAGMENTS);
if (fragments.getCount() != 0) {
MapBuffer fragment = fragments.getMapBuffer(0);
MapBuffer textAttributes = fragment.getMapBuffer(FR_KEY_TEXT_ATTRIBUTES);
if (alignmentAttr == null) {
return alignment;
}

if (textAttributes.contains(TextAttributeProps.TA_KEY_ALIGNMENT)) {
String alignmentAttr = textAttributes.getString(TextAttributeProps.TA_KEY_ALIGNMENT);

if (alignmentAttr.equals("center")) {
alignment = Layout.Alignment.ALIGN_CENTER;
} else if (alignmentAttr.equals("right")) {
alignment =
swapNormalAndOpposite
? Layout.Alignment.ALIGN_NORMAL
: Layout.Alignment.ALIGN_OPPOSITE;
}
}
if (alignmentAttr.equals("center")) {
alignment = Layout.Alignment.ALIGN_CENTER;
} else if (alignmentAttr.equals("right")) {
alignment =
swapNormalAndOpposite ? Layout.Alignment.ALIGN_NORMAL : Layout.Alignment.ALIGN_OPPOSITE;
}

return alignment;
Expand All @@ -190,7 +210,8 @@ private static Layout.Alignment getTextAlignment(MapBuffer attributedString, Spa
public static int getTextGravity(
MapBuffer attributedString, Spannable spanned, int defaultValue) {
int gravity = defaultValue;
Layout.Alignment alignment = getTextAlignment(attributedString, spanned);
@Nullable String alignmentAttr = getTextAlignmentAttr(attributedString);
Layout.Alignment alignment = getTextAlignment(attributedString, spanned, alignmentAttr);

// depending on whether the script is LTR or RTL, ALIGN_NORMAL and ALIGN_OPPOSITE may mean
// different things
Expand Down Expand Up @@ -363,6 +384,7 @@ private static Layout createLayout(
int textBreakStrategy,
int hyphenationFrequency,
Layout.Alignment alignment,
int justificationMode,
TextPaint paint) {
Layout layout;

Expand Down Expand Up @@ -420,6 +442,10 @@ private static Layout createLayout(
.setTextDirection(
isScriptRTL ? TextDirectionHeuristics.RTL : TextDirectionHeuristics.LTR);

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) {
builder.setJustificationMode(justificationMode);
}

if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
builder.setUseLineSpacingFromFallbacks(true);
}
Expand Down Expand Up @@ -506,7 +532,9 @@ private static Layout createLayout(
? paragraphAttributes.getInt(PA_KEY_MAX_NUMBER_OF_LINES)
: ReactConstants.UNSET;

Layout.Alignment alignment = getTextAlignment(attributedString, text);
@Nullable String alignmentAttr = getTextAlignmentAttr(attributedString);
Layout.Alignment alignment = getTextAlignment(attributedString, text, alignmentAttr);
int justificationMode = getTextJustificationMode(alignmentAttr);

if (adjustFontSizeToFit) {
double minimumFontSize =
Expand All @@ -526,6 +554,7 @@ private static Layout createLayout(
textBreakStrategy,
hyphenationFrequency,
alignment,
justificationMode,
paint);
}

Expand All @@ -538,6 +567,7 @@ private static Layout createLayout(
textBreakStrategy,
hyphenationFrequency,
alignment,
justificationMode,
paint);
}

Expand All @@ -553,6 +583,7 @@ private static Layout createLayout(
int textBreakStrategy,
int hyphenationFrequency,
Layout.Alignment alignment,
int justificationMode,
TextPaint paint) {
BoringLayout.Metrics boring = BoringLayout.isBoring(text, paint);
Layout layout =
Expand All @@ -565,6 +596,7 @@ private static Layout createLayout(
textBreakStrategy,
hyphenationFrequency,
alignment,
justificationMode,
paint);

// Minimum font size is 4pts to match the iOS implementation.
Expand Down Expand Up @@ -613,6 +645,7 @@ private static Layout createLayout(
textBreakStrategy,
hyphenationFrequency,
alignment,
justificationMode,
paint);
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/rn-tester/js/examples/Text/TextExample.android.js
Original file line number Diff line number Diff line change
Expand Up @@ -696,7 +696,7 @@ function NestedExample(props: {}): React.Node {

function TextAlignExample(props: {}): React.Node {
return (
<>
<View testID="text-align-example">
<Text>auto (default) - english LTR</Text>
<Text>أحب اللغة العربية auto (default) - arabic RTL</Text>
<Text style={{textAlign: 'left'}}>
Expand All @@ -717,7 +717,7 @@ function TextAlignExample(props: {}): React.Node {
as you can see all of the lines except the last one span the available
width of the parent container.
</Text>
</>
</View>
);
}

Expand Down
5 changes: 4 additions & 1 deletion packages/rn-tester/js/examples/Text/TextExample.ios.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ class TextAlignRTLExample extends React.Component<
const {isRTL} = this.state;
const toggleRTL = () => this.setState({isRTL: !isRTL});
return (
<View style={{direction: isRTL ? 'rtl' : 'ltr'}}>
<View
style={{direction: isRTL ? 'rtl' : 'ltr'}}
testID="text-align-example">
<Text>auto (default) - english LTR</Text>
<Text>
{'\u0623\u062D\u0628 \u0627\u0644\u0644\u063A\u0629 ' +
Expand Down Expand Up @@ -1254,6 +1256,7 @@ const examples = [
},
{
title: 'Text Align with RTL',
name: 'textAlign',
render: function (): React.Node {
return <TextAlignRTLExample />;
},
Expand Down

0 comments on commit 08e8f6a

Please sign in to comment.