[Android] textAlignVertical doesn't work on nested text

jsamr commented 4 years ago

This issue has already been reported (#18790), but closed for "stalling". As of React Native 0.63.3, the bug still exists.

Description

textAlignVertical works fine when set on the top-most Text component. However, if you have any nested texts (a common use case), you can not override the alignVertical in the child Text component.

React Native version:

System: OS: Linux 5.4 Manjaro Linux CPU: (8) x64 Intel(R) Core(TM) i7-8809G CPU @ 3.10GHz Memory: 1.54 GB / 31.29 GB Shell: 5.8 - /bin/zsh Binaries: Node: 14.15.0 - /tmp/xfs-2fbb69bb/node Yarn: 2.2.2 - /tmp/xfs-2fbb69bb/yarn npm: 6.14.8 - /usr/bin/npm Watchman: 4.9.0 - /usr/bin/watchman SDKs: Android SDK: API Levels: 28, 29 Build Tools: 28.0.3, 29.0.0, 29.0.2, 29.0.3, 30.0.0, 30.0.1 System Images: android-22 | Google APIs Intel x86 Atom, android-23 | Google APIs Intel x86 Atom, android-29 | Google APIs Intel x86 Atom, android-30 | Google Play Intel x86 Atom Android NDK: Not Found IDEs: Android Studio: 4.1 AI-201.8743.12.41.6858069 Languages: Java: 1.8.0_265 - /usr/bin/javac Python: 3.8.6 - /usr/bin/python npmPackages: @react-native-community/cli: Not Found react: 16.13.1 => 16.13.1 react-native: ^0.63.3 => 0.63.3 npmGlobalPackages: react-native: Not Found

Steps To Reproduce

Create an app which renders the following component:

export default function App() {
  return (
      <Text>
        <Text>This is text</Text>
        <Text style={{textAlignVertical: 'top', fontSize: 10}}>sup!</Text>
      </Text>
  );
}

Expected Results

The "sup!" Text node should be aligned to the top edge of the line. Instead, "sup!" is aligned with the baseline:

Web (expected)

2020-11-12-141856_128x45_scrot

Android

2020-11-12-141936_167x69_scrot

Snack, code example, screenshot, or link to a repository:

https://snack.expo.io/@jsamr/android-textalignvertical

dededavida commented 3 years ago

hey guys, I created a component that worked for me, I'm sharing https://github.com/daviddossantos/textinput-multiline-multiplataform

jsamr commented 3 years ago

@daviddossantos thanks for sharing but this is unrelated! The bug I reported was on Android and targeted Textcomponent. Your workaround seems to target iOS and the TextInputcomponent.

dededavida commented 3 years ago

sorry, I didn't pay attention

fabOnReact commented 3 years ago

unluckily @hoda-oz is right

Even if Text components are nested in other Text components, they are merged into a single TextView of Android So you cannot manipulate text position in nested Text components. In this case, you can manipulate vertical position by lineHeight property.

I have been reading https://github.com/facebook/react-native/pull/23195/files and https://github.com/facebook/react-native/pull/8619/files to understand how nested Text are combined in one TextView

<Text>
  <Text>This is text</Text>
  <Text style={{textAlignVertical: 'top', fontSize: 10, backgroundColor: "red" }}>sup!</Text>
</Text>

https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L42-L47

The code below loops the different text and probably is responsible for aggregating them in 1 TextView.

https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L111-L112

each one of the below code-snippets adds a different text property to the Text sup! using the Android Spans API. Hopefully I will be able to use AlignmentSpan to add the required functionality...

More code references

https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L174 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L178-L179 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L186 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L193 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L199-L207 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L210 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L213 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L220-L227 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L233 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L235

fabOnReact commented 3 years ago

the line below changes the text sup! to background color red

https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L176-L180

fabOnReact commented 3 years ago

each one of the below code-snippets adds a different text property to the Text sup! using the Android Spans API. Hopefully I will be able to use AlignmentSpan to add the required functionality...

CLICK TO OPEN CODE SNIPPETS

https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L174 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L178-L179 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L186 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L193 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L199-L207 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L210 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L213 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L220-L227 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L233 https://github.com/facebook/react-native/blob/28fb41a0ab48cc01d606b64744c84e2ac3805f3f/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L235

fabOnReact commented 3 years ago

I further researched and found the AlignmentSpan

as explained in ParagraphSpans they need a new line \n like Spantastic\ntext.

as explained in AlignmentSpan.Standard we can use the following Layout.Alignment

ALIGN_CENTER
ALIGN_OPPOSITE
ALIGN_NORMAL

This are examples of ALIGN_OPPOSITE

The functionality is not available in AOSP so I should create a CustomSpan which inherits from AlignmentSpan and changes the text-alignment to top.

AlignmentSpan uses mAlignment.name() to retrieve the Span with the correct alignment

https://github.com/aosp-mirror/platform_frameworks_base/blob/7096deadf4a32768717e74b11eb8e6289a65976f/core/java/android/text/style/AlignmentSpan.java#L95-L98

public interface AlignmentSpan extends ParagraphStyle {
    class Standard implements AlignmentSpan, ParcelableSpan {
        @Override
        public void writeToParcelInternal(@NonNull Parcel dest, int flags) {
            dest.writeString(mAlignment.name());
        }
   }
}

https://github.com/aosp-mirror/platform_frameworks_base/blob/7096deadf4a32768717e74b11eb8e6289a65976f/core/java/android/text/Layout.java#L529-L581

https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/text/Layout.java;l=440;drc=caabd536e8fea82b888902f359b33e637267966c?q=ALIGN_OPPOSITE&ss=android%2Fplatform%2Fsuperproject

CLICK TO OPEN ANDROID SOURCECODE

```java public abstract class Layout { @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553) public void drawText(Canvas canvas, int firstLine, int lastLine) { // not relevant logic // Determine whether the line aligns to normal, opposite, or center. Alignment align = paraAlign; if (align == Alignment.ALIGN_LEFT) { align = (dir == DIR_LEFT_TO_RIGHT) ? Alignment.ALIGN_NORMAL : Alignment.ALIGN_OPPOSITE; } else if (align == Alignment.ALIGN_RIGHT) { align = (dir == DIR_LEFT_TO_RIGHT) ? Alignment.ALIGN_OPPOSITE : Alignment.ALIGN_NORMAL; } int x; final int indentWidth; if (align == Alignment.ALIGN_NORMAL) { if (dir == DIR_LEFT_TO_RIGHT) { indentWidth = getIndentAdjust(lineNum, Alignment.ALIGN_LEFT); x = left + indentWidth; } else { indentWidth = -getIndentAdjust(lineNum, Alignment.ALIGN_RIGHT); x = right - indentWidth; } } else { int max = (int)getLineExtent(lineNum, tabStops, false); if (align == Alignment.ALIGN_OPPOSITE) { if (dir == DIR_LEFT_TO_RIGHT) { indentWidth = -getIndentAdjust(lineNum, Alignment.ALIGN_RIGHT); x = right - max - indentWidth; } else { indentWidth = getIndentAdjust(lineNum, Alignment.ALIGN_LEFT); x = left - max + indentWidth; } } else { // Alignment.ALIGN_CENTER indentWidth = getIndentAdjust(lineNum, Alignment.ALIGN_CENTER); max = max & ~1; x = ((right + left - max) >> 1) + indentWidth; } } Directions directions = getLineDirections(lineNum); if (directions == DIRS_ALL_LEFT_TO_RIGHT && !mSpannedText && !hasTab && !justify) { // XXX: assumes there's nothing additional to be done canvas.drawText(buf, start, end, x, lbaseline, paint); } else { tl.set(paint, buf, start, end, dir, directions, hasTab, tabStops, getEllipsisStart(lineNum), getEllipsisStart(lineNum) + getEllipsisCount(lineNum)); if (justify) { tl.justify(right - left - indentWidth); } tl.draw(canvas, x, ltop, lbaseline, lbottom); } } TextLine.recycle(tl); } ```

jsamr commented 3 years ago

@fabriziobertoglio1987 Awesome! Thanks for your research. Looking forwards to your PR :slightly_smiling_face:

fabOnReact commented 3 years ago

@jsamr thanks a lot Jules Randolph for the sponsorship. I will give priority to this issue and publish a pull request. :pray: :peace_symbol: :heart:

fabOnReact commented 3 years ago

thanks a lot @jsamr

Yes, I believe you are right .. We can use SuperscriptSpan and SubscriptSpan

CLICK TO OPEN TESTS RESULTS

TEST SCENARIO - we use [SuperscriptSpan](https://developer.android.com/reference/android/text/style/SuperscriptSpan) to top align the text RESULT - the text is top aligned ```javascript Parent Solid underline ``` in ReactBaseTextShadowNode ```java if (textShadowNode.mTextAlignVertical == "super") { ops.add(new SetSpanOperation(start, end, new ReactSuperscriptSpan())); } ``` Superscript Subscript

jsamr commented 3 years ago

@fabriziobertoglio1987 That is promising! Although it would be more like super / sub support rather than a fix for this peculiar issue. But a very exciting new feature! Below just a recap on the distinction between super and top.

In CSS, top, bottom are quite different than sub and sup:

The baseline-relative values %, sub, super shift the box relative to its baseline-aligned position, whereas the line-relative values top, center, and bottom shift the inline box and its contents relative to the bounds of its line box.

What that basically mean is that, if you have a sibling which expands the line box such as a big image, top would put the content aligned with the top of the image (which coincide with the line box) while super would only shift the content a little bit up from the baseline.

See this JSFiddle

But honestly, I have no idea what are the specs of React Native inline layout! There certainly are models for native platforms.

fabOnReact commented 3 years ago

I was able to prepare a fix for this issue, the problem is that I don't know how to pass the prop. The prop textAlignVertical: 'super' needs to be passed to the span (the nested/child <Text>sup!</Text>) and not the parent android TextView (the parent react <Text>This is text</Text>).

The class that handle TextView attributes is ReactTextAnchorViewManager which for example applies the prop TEXT_ALIGN_VERTICAL and changes the alignment of the parent Text.

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextAnchorViewManager.java#L72

while the second child/nexted text <Text>This is text<Text>sup!</Text></Text> is just part of the original TextView, but it is styled differently using an android SuperscriptSpan insted of for example CustomLetterSpacingSpan

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L176-L180

This properties are applied by the ReactBaseTextShadowNode and are taked from the textAttributes properties listed below

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L323-L330

I still did not find the logic that adds this properties to the ReactBaseTextShadowNode, but I imagine may be in the ReactTextViewAnchorManager

I will have to further investigate were to add this property, probably I will have to investigate how the style properties are used to add the spans. Thanks a lot for the patience in adding this feature :pray:

fabOnReact commented 3 years ago

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java#L109-L111

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java#L242-L245

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L478-L491

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L176-L180

jsamr commented 3 years ago

@fabriziobertoglio1987 Any updates?

fabOnReact commented 3 years ago

I'm searching for a way to pass the props to the span Android element so that it is displayed as super or subscript.

As explained in the notes above, I analyzed the way the style prop backgroundColor is passed:

1) textAttributes includes the backgroundColor passed with the style prop.

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java#L109-L111

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java#L137-L141

2) explanation of how in point 1) the textAttributes variables includes also the style.backgroundColor value. The method TextAttributesProps.fromReadableMap retrieves the backgroundColor and saves it in result variable.

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/TextAttributeProps.java#L242-L245

3) in the case of ReactBaseTextShadowNode the textAttributes are taken from the parentTextAttributes

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L104-L109

4) a setter is available to update the backgroundColor

https://github.com/facebook/react-native/blob/cf0a6e9e2734a5927b9dc08303c22477ad274150/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L478-L491

I will try to add the prop super/subscript to the textAttributes. Thanks a lot for the support. I will update you and try to prepare the pull request for today.

fabOnReact commented 3 years ago

The backgroundColor is saved in textShadowNode.mBackgroundColor

https://github.com/facebook/react-native/blob/40a93914c69fc46cd4c7ba0ed025a1f4f29453ff/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactBaseTextShadowNode.java#L96

CLICK TO OPEN TESTS RESULTS

FLog.w("TESTING::ReactBaseTextShadowNode", "text: " + (text));
FLog.w("TESTING::ReactBaseTextShadowNode", "textShadowNode.mIsBackgroundColorSet: " + (textShadowNode.mIsBackgroundColorSet));
String hexColor = String.format("#%06X", (0xFFFFFF & textShadowNode.mBackgroundColor));
FLog.w("TESTING::ReactBaseTextShadowNode", "textShadowNode.mBackgroundColor: " + (hexColor));

logcat return

07-01 07:44:54.972  5186  5238 W unknown:TESTING::ReactBaseTextShadowNode: text: Parent
07-01 07:44:54.972  5186  5238 W unknown:TESTING::ReactBaseTextShadowNode: textShadowNode.mIsBackgroundColorSet: false
07-01 07:44:54.972  5186  5238 W unknown:TESTING::ReactBaseTextShadowNode: textShadowNode.mBackgroundColor: #000000
07-01 07:44:54.972  5186  5238 W unknown:TESTING::ReactBaseTextShadowNode: text: Child
07-01 07:44:54.972  5186  5238 W unknown:TESTING::ReactBaseTextShadowNode: textShadowNode.mIsBackgroundColorSet: true
07-01 07:44:54.972  5186  5238 W unknown:TESTING::ReactBaseTextShadowNode: textShadowNode.mBackgroundColor: #0000FF

I will continue investigating how to pass props through textShadowNode

Svarto commented 2 years ago

Following up here, still encounter this issue. Any update? Any workaround?

fabOnReact commented 1 year ago

As discussed in this comment in the conversation 2022: How can we improve React Native? #528, I'm starting to work on this issue, and I will try to publish a PR. A summary of all the updates will be posted in https://github.com/react-native-community/discussions-and-proposals/issues/495#issuecomment-1363027413

fabOnReact commented 1 year ago

@fabriziobertoglio1987 That is promising! Although it would be more like super / sub support rather than a fix for this peculiar issue. But a very exciting new feature! Below just a recap on the distinction between super and top.

In CSS, top, bottom are quite different than sub and sup:

The baseline-relative values %, sub, super shift the box relative to its baseline-aligned position, whereas the line-relative values top, center, and bottom shift the inline box and its contents relative to the bounds of its line box.

What that basically mean is that, if you have a sibling which expands the line box such as a big image, top would put the content aligned with the top of the image (which coincide with the line box) while super would only shift the content a little bit up from the baseline.

See this JSFiddle

But honestly, I have no idea what are the specs of React Native inline layout! There certainly are models for native platforms.

@jsamr in react-native we use gravity to align the parent text top/center/bottom Android Gravity behaves like flex box https://github.com/facebook/react-native/pull/35704#issuecomment-1369192954

For this reason a Text with height 250px and textAlignVertical bottom aligns to the bottom (baseline -250px). CSS behaves different from react-native. The same result is obtained with flex style instead of vertical-align

I think the expected behaviour for this functionality is aligning the text relatively to the lineHeight and not using Gravity (alignment relative to the parent component) https://jsfiddle.net/Lbc0y7ph/

https://www.w3schools.com/cssref/pr_pos_vertical-align.php

I will update my existing solution, which somehow implements the same flex layout logic to nested text (align nested text top/bottom of the parent component).

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

miallo commented 1 year ago

Still an issue

paulschreiber commented 12 months ago

Still a problem with 0.72.6.

fabOnReact commented 10 months ago

Hello,

Thanks for the issue. I have been contributing to facebook/react-native for 4 years and specialize in the Text/TextInput components. I currently have 58 facebook/react-native PRs:

https://github.com/facebook/react-native/pulls?q=is%3Apr+author%3Afabriziobertoglio1987+

This is the suggested approach from the react-native core team (see this discussion):

I'm publishing several fixes for Text and TextInput component in a separate library react-native-improved.

The Component is based on ReactTextView, it is the same implementation from react-native.
It will include several bug fixes.
You can use the library until the PR is merged and released with react-native.
You can use side by side both components from react-native and react-native-improved, depending on where you need the fix.
I will also publish the PR to facebook/react-native (daily fixes and releases).

The advantages would be:

Increased engagement with the community, issues are quickly fixed.
No limitations when merging PRs (react-native main branch is used on facebook marketplace).
Only Meta employees can merge and review facebook/react-native PRs.
Allows us to further experiment and develop react-native.

What do you think about this? Should I keep working on this? Thanks

fabOnReact commented 10 months ago

I'm the author of https://github.com/facebook/react-native/pull/35949 and https://github.com/facebook/react-native/pull/35704. The two PRs fix this issue.

ngoclamnn commented 7 months ago

still getting this issue when render sub and sup html tag using react-native-render-html

qwertychouskie commented 1 month ago

@fabOnReact Both those PRs were closed as stale. Are there new PRs or is the project stalled?

facebook / react-native