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

[ANDROID] [FIXED] Fixed random styling for text nodes with many children #36656

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -70,31 +70,6 @@ public abstract class ReactBaseTextShadowNode extends LayoutShadowNode {

protected @Nullable ReactTextViewManagerCallback mReactTextViewManagerCallback;

private static class SetSpanOperation {
protected int start, end;
protected ReactSpan what;

SetSpanOperation(int start, int end, ReactSpan what) {
this.start = start;
this.end = end;
this.what = what;
}

public void execute(SpannableStringBuilder sb, int priority) {
// All spans will automatically extend to the right of the text, but not the left - except
// for spans that start at the beginning of the text.
int spanFlags = Spannable.SPAN_EXCLUSIVE_INCLUSIVE;
if (start == 0) {
spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE;
}

spanFlags &= ~Spannable.SPAN_PRIORITY;
spanFlags |= (priority << Spannable.SPAN_PRIORITY_SHIFT) & Spannable.SPAN_PRIORITY;

sb.setSpan(what, start, end, spanFlags);
}
}

private static void buildSpannedFromShadowNode(
ReactBaseTextShadowNode textShadowNode,
SpannableStringBuilder sb,
Expand Down Expand Up @@ -281,8 +256,9 @@ protected Spannable spannedFromShadowNode(

// While setting the Spans on the final text, we also check whether any of them are inline views
// or images.
int priority = 0;
for (SetSpanOperation op : ops) {
for (int priorityIndex = 0; priorityIndex < ops.size(); priorityIndex++) {
final SetSpanOperation op = ops.get(ops.size() - priorityIndex - 1);

boolean isInlineImage = op.what instanceof TextInlineImageSpan;
if (isInlineImage || op.what instanceof TextInlineViewPlaceholderSpan) {
int height;
Expand All @@ -309,9 +285,8 @@ protected Spannable spannedFromShadowNode(
}

// Actual order of calling {@code execute} does NOT matter,
// but the {@code priority} DOES matter.
op.execute(sb, priority);
priority++;
// but the {@code priorityIndex} DOES matter.
op.execute(sb, priorityIndex);
}

textShadowNode.mTextAttributes.setHeightOfTallestInlineViewOrImage(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.facebook.react.views.text;

import android.text.Spannable;
import android.text.SpannableStringBuilder;
import android.text.Spanned;

import com.facebook.common.logging.FLog;

class SetSpanOperation {
private static final String TAG = "SetSpanOperation";
static final int SPAN_MAX_PRIORITY = Spanned.SPAN_PRIORITY >> Spanned.SPAN_PRIORITY_SHIFT;

protected int start, end;
protected ReactSpan what;

SetSpanOperation(int start, int end, ReactSpan what) {
this.start = start;
this.end = end;
this.what = what;
}

/**
* @param sb builder
* @param priorityIndex index of this operation in the topological sorting which puts operations
* with higher priority before operations with lower priority.
*/
public void execute(SpannableStringBuilder sb, int priorityIndex) {
assert priorityIndex >= 0;

// All spans will automatically extend to the right of the text, but not the left - except
// for spans that start at the beginning of the text.
int spanFlags = Spannable.SPAN_EXCLUSIVE_INCLUSIVE;
if (start == 0) {
spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE;
}

// Calculate priority, assigning the highest values to operations with the highest priority
final int priority = SPAN_MAX_PRIORITY - priorityIndex;

if (priority < 0) {
FLog.w(TAG, "Text tree size exceeded the limit, styling may become unpredictable");
}

// If the computed priority doesn't fit in the flags, clamp it. The effect might not be correct
// in 100% of cases, but doing nothing (as we did in the past) leads to totally random results.
final int effectivePriority = Math.max(priority, 0);

spanFlags &= ~Spannable.SPAN_PRIORITY;
spanFlags |= (effectivePriority << Spannable.SPAN_PRIORITY_SHIFT) & Spannable.SPAN_PRIORITY;

sb.setSpan(what, start, end, spanFlags);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -223,12 +223,12 @@ private static Spannable createSpannableFromAttributedString(

// TODO T31905686: add support for inline Images
// While setting the Spans on the final text, we also check whether any of them are images.
int priority = 0;
for (SetSpanOperation op : ops) {
for (int priorityIndex = 0; priorityIndex < ops.size(); ++priorityIndex) {
final SetSpanOperation op = ops.get(ops.size() - priorityIndex - 1);

// Actual order of calling {@code execute} does NOT matter,
// but the {@code priority} DOES matter.
op.execute(sb, priority);
priority++;
// but the {@code priorityIndex} DOES matter.
op.execute(sb, priorityIndex);
}

if (reactTextViewManagerCallback != null) {
Expand Down Expand Up @@ -561,30 +561,4 @@ public static WritableArray measureLines(
hyphenationFrequency);
return FontMetricsUtil.getFontMetrics(text, layout, sTextPaintInstance, context);
}

// TODO T31905686: This class should be private
public static class SetSpanOperation {
protected int start, end;
protected ReactSpan what;

public SetSpanOperation(int start, int end, ReactSpan what) {
this.start = start;
this.end = end;
this.what = what;
}

public void execute(Spannable sb, int priority) {
// All spans will automatically extend to the right of the text, but not the left - except
// for spans that start at the beginning of the text.
int spanFlags = Spannable.SPAN_EXCLUSIVE_INCLUSIVE;
if (start == 0) {
spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE;
}

spanFlags &= ~Spannable.SPAN_PRIORITY;
spanFlags |= (priority << Spannable.SPAN_PRIORITY_SHIFT) & Spannable.SPAN_PRIORITY;

sb.setSpan(what, start, end, spanFlags);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -264,12 +264,12 @@ private static Spannable createSpannableFromAttributedString(

// TODO T31905686: add support for inline Images
// While setting the Spans on the final text, we also check whether any of them are images.
int priority = 0;
for (SetSpanOperation op : ops) {
for (int priorityIndex = 0; priorityIndex < ops.size(); ++priorityIndex) {
final SetSpanOperation op = ops.get(ops.size() - priorityIndex - 1);

// Actual order of calling {@code execute} does NOT matter,
// but the {@code priority} DOES matter.
op.execute(sb, priority);
priority++;
// but the {@code priorityIndex} DOES matter.
op.execute(sb, priorityIndex);
}

if (reactTextViewManagerCallback != null) {
Expand Down Expand Up @@ -588,30 +588,4 @@ public static WritableArray measureLines(
hyphenationFrequency);
return FontMetricsUtil.getFontMetrics(text, layout, sTextPaintInstance, context);
}

// TODO T31905686: This class should be private
cubuspl42 marked this conversation as resolved.
Show resolved Hide resolved
public static class SetSpanOperation {
protected int start, end;
protected ReactSpan what;

public SetSpanOperation(int start, int end, ReactSpan what) {
this.start = start;
this.end = end;
this.what = what;
}

public void execute(Spannable sb, int priority) {
// All spans will automatically extend to the right of the text, but not the left - except
// for spans that start at the beginning of the text.
int spanFlags = Spannable.SPAN_EXCLUSIVE_INCLUSIVE;
if (start == 0) {
spanFlags = Spannable.SPAN_INCLUSIVE_INCLUSIVE;
}

spanFlags &= ~Spannable.SPAN_PRIORITY;
spanFlags |= (priority << Spannable.SPAN_PRIORITY_SHIFT) & Spannable.SPAN_PRIORITY;

sb.setSpan(what, start, end, spanFlags);
}
}
}
Loading