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] textAlignVertical doesn't work on nested text #30375

Open
jsamr opened this issue Nov 12, 2020 · 26 comments
Open

[Android] textAlignVertical doesn't work on nested text #30375

jsamr opened this issue Nov 12, 2020 · 26 comments
Labels
Needs: Triage 🔍 Never gets stale Prevent those issues and PRs from getting stale Platform: Android Android applications. Platform: Linux Building on Linux.

Comments

@jsamr
Copy link

jsamr commented Nov 12, 2020

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

@react-native-bot react-native-bot added Platform: Android Android applications. Platform: Linux Building on Linux. labels Nov 12, 2020
@dededavida
Copy link

dededavida commented Jan 27, 2021

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

@jsamr
Copy link
Author

jsamr commented Jan 27, 2021

@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
Copy link

sorry, I didn't pay attention

@fabOnReact
Copy link
Contributor

fabOnReact commented Feb 18, 2021

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>

* <p>This class handles all text attributes associated with {@code <Text>}-ish node. A concrete
* node can be an anchor {@code <Text>} node, an anchor {@code <TextInput>} node or virtual {@code
* <Text>} node inside {@code <Text>} or {@code <TextInput>} node. Or even something else.
*
* <p>This also node calculates {@link Spannable} object based on subnodes of the same type, which
* can be used in concrete classes to feed native views and compute layout.

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

for (int i = 0, length = textShadowNode.getChildCount(); i < length; i++) {
ReactShadowNode child = textShadowNode.getChildAt(i);

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

new SetSpanOperation(start, end, new ReactForegroundColorSpan(textShadowNode.mColor)));

new SetSpanOperation(
start, end, new ReactBackgroundColorSpan(textShadowNode.mBackgroundColor)));

new SetSpanOperation(start, end, new CustomLetterSpacingSpan(effectiveLetterSpacing)));

ops.add(new SetSpanOperation(start, end, new ReactAbsoluteSizeSpan(effectiveFontSize)));

new SetSpanOperation(
start,
end,
new CustomStyleSpan(
textShadowNode.mFontStyle,
textShadowNode.mFontWeight,
textShadowNode.mFontFeatureSettings,
textShadowNode.mFontFamily,
textShadowNode.getThemedContext().getAssets())));

ops.add(new SetSpanOperation(start, end, new ReactUnderlineSpan()));

ops.add(new SetSpanOperation(start, end, new ReactStrikethroughSpan()));

new SetSpanOperation(
start,
end,
new ShadowStyleSpan(
textShadowNode.mTextShadowOffsetDx,
textShadowNode.mTextShadowOffsetDy,
textShadowNode.mTextShadowRadius,
textShadowNode.mTextShadowColor)));

ops.add(new SetSpanOperation(start, end, new CustomLineHeightSpan(effectiveLineHeight)));

ops.add(new SetSpanOperation(start, end, new ReactTagSpan(textShadowNode.getReactTag())));

@fabOnReact

This comment has been minimized.

@fabOnReact

This comment was marked as duplicate.

@fabOnReact
Copy link
Contributor

fabOnReact commented Feb 19, 2021

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

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

This comment has been minimized.

@fabOnReact

This comment has been minimized.

@fabOnReact
Copy link
Contributor

fabOnReact commented Apr 17, 2021

thanks a lot @jsamr

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

CLICK TO OPEN TESTS RESULTS

TEST SCENARIO

RESULT

  • the text is top aligned
 <Text>
  Parent
  <Text style={{textAlignVertical: 'superscript'}}>
    Solid underline
  </Text>
</Text>

in ReactBaseTextShadowNode

if (textShadowNode.mTextAlignVertical == "super") {
  ops.add(new SetSpanOperation(start, end, new ReactSuperscriptSpan()));
}

Superscript

Subscript

@jsamr
Copy link
Author

jsamr commented Apr 17, 2021

@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
Copy link
Contributor

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.

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

if (textShadowNode.mIsBackgroundColorSet) {
ops.add(
new SetSpanOperation(
start, end, new ReactBackgroundColorSpan(textShadowNode.mBackgroundColor)));
}

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

protected int mNumberOfLines = UNSET;
protected int mTextAlign = Gravity.NO_GRAVITY;
protected int mTextBreakStrategy =
(Build.VERSION.SDK_INT < Build.VERSION_CODES.M) ? 0 : Layout.BREAK_STRATEGY_HIGH_QUALITY;
protected int mHyphenationFrequency =
(Build.VERSION.SDK_INT < Build.VERSION_CODES.M) ? 0 : Layout.HYPHENATION_FREQUENCY_NONE;
protected int mJustificationMode =
(Build.VERSION.SDK_INT < Build.VERSION_CODES.O) ? 0 : Layout.JUSTIFICATION_MODE_NONE;

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 🙏

@fabOnReact
Copy link
Contributor

fabOnReact commented May 3, 2021

TextAttributeProps textAttributes =
TextAttributeProps.fromReadableMap(
new ReactStylesDiffMap(fragment.getMap("textAttributes")));

result.setBackgroundColor(
props.hasKey(ViewProps.BACKGROUND_COLOR)
? props.getInt(ViewProps.BACKGROUND_COLOR, 0)
: null);

@ReactProp(name = ViewProps.BACKGROUND_COLOR, customType = "Color")
public void setBackgroundColor(@Nullable Integer color) {
// Background color needs to be handled here for virtual nodes so it can be incorporated into
// the span. However, it doesn't need to be applied to non-virtual nodes because non-virtual
// nodes get mapped to native views and native views get their background colors get set via
// {@link BaseViewManager}.
if (isVirtual()) {
mIsBackgroundColorSet = (color != null);
if (mIsBackgroundColorSet) {
mBackgroundColor = color;
}
markUpdated();
}
}

if (textShadowNode.mIsBackgroundColorSet) {
ops.add(
new SetSpanOperation(
start, end, new ReactBackgroundColorSpan(textShadowNode.mBackgroundColor)));
}

@jsamr
Copy link
Author

jsamr commented Jun 30, 2021

@fabriziobertoglio1987 Any updates?

@fabOnReact
Copy link
Contributor

fabOnReact commented Jul 1, 2021

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.

TextAttributeProps textAttributes =
TextAttributeProps.fromReadableMap(
new ReactStylesDiffMap(fragment.getMap("textAttributes")));

if (textAttributes.mIsBackgroundColorSet) {
ops.add(
new SetSpanOperation(
start, end, new ReactBackgroundColorSpan(textAttributes.mBackgroundColor)));
}

  1. 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.

result.setBackgroundColor(
props.hasKey(ViewProps.BACKGROUND_COLOR)
? props.getInt(ViewProps.BACKGROUND_COLOR, 0)
: null);

  1. in the case of ReactBaseTextShadowNode the textAttributes are taken from the parentTextAttributes

TextAttributes textAttributes;
if (parentTextAttributes != null) {
textAttributes = parentTextAttributes.applyChild(textShadowNode.mTextAttributes);
} else {
textAttributes = textShadowNode.mTextAttributes;
}

  1. a setter is available to update the backgroundColor

@ReactProp(name = ViewProps.BACKGROUND_COLOR, customType = "Color")
public void setBackgroundColor(@Nullable Integer color) {
// Background color needs to be handled here for virtual nodes so it can be incorporated into
// the span. However, it doesn't need to be applied to non-virtual nodes because non-virtual
// nodes get mapped to native views and native views get their background colors get set via
// {@link BaseViewManager}.
if (isVirtual()) {
mIsBackgroundColorSet = (color != null);
if (mIsBackgroundColorSet) {
mBackgroundColor = color;
}
markUpdated();
}
}

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
Copy link
Contributor

fabOnReact commented Jul 1, 2021

The backgroundColor is saved in textShadowNode.mBackgroundColor


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
Copy link

Svarto commented Nov 27, 2021

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

facebook-github-bot pushed a commit that referenced this issue Mar 29, 2022
Summary:
This issue fixes [32004][23]. The Pull Request was previously published by [blavalla][10] with [31757][24].
>This is a follow-up on [D23553222 (https://github.com/facebook/react-native/commit/b352e2da8137452f66717cf1cecb2e72abd727d7)][18], which made links functional by using [Talkback's Links menu][1]. We don't often use this as the sole access point for links due to it being more difficult for users to navigate to and easy for users to miss if they don't listen to the full description, including the hint text that announces that links are available.
The Implementation of the functionality consists of:

Retrieving the accessibility links and triggering the TalkBack Focus over the Text
1. nested Text components with accessibilityRole link are saved as [ReactClickableSpan][17] instances in Android native [TextView][20] ([more info][19])
1. If the TextView contains any [ClickableSpans][15] (which are [nested Text][14] components with role link), set a view tag and reset the accessibility delegate.
3. Obtain each link description, start, end, and position relative to the parent Text (id) from the Span as an [AccessibilityLink][16]
4. Use the [AccessibilityLink][16]  to display TalkBack focus over the link with the `getVirtualViewAt` method (more [info][13])

Implementing ExploreByTouchHelper to detect touches over links and to display TalkBack rectangle around them.
1. ReactAccessibilityDelegate inherits from [ExploreByTouchHelper][12]
2. If the [ReactTextView][21] has an accessibility delegate, trigger ExploreByTouchHelper method [dispatchHoverEvent][22]
3.  Implements the methods `getVirtualViewAt` and `onPopulateBoundsForVirtualView`.
     The two methods implements the following functionalities  (more [info][13]):
    * detecting the TalkBack onPress/focus on nested Text with accessibilityRole="link"
    * displaying TalkBack rectangle around nested Text with accessibilityRole="link"

## Changelog

[Android] [Added] - Make links independently focusable by Talkback

Pull Request resolved: #33215

Test Plan:
[1]. User Interacts with links through TalkBack default accessibility menu ([link][1])
[2]. The nested link becomes the next focusable element after the parent element that contains it. ([link][2])
[3]. Testing accessibility examples in pr branch ([link][3])
[4]. Testing accessibility android examples in pr branch ([link][4])
[7]. TalkBack focus moves through links in the correct order from top to bottom (PR Branch with [link.id][25]) ([link to video test][7]) ([discussion][26])
[8]. TalkBack focus does not move through links in the correct order from top to bottom (PR Branch without [link.id][25]) ([link to video test][8]) ([discussion][26])

Test on main branch
[5]. Testing accessibility examples in main branch ([link][5])
[6]. Testing accessibility android examples in main branch ([link][6])

[1]: fabOnReact/react-native-notes#9 (comment)
[2]: fabOnReact/react-native-notes#9 (comment)
[3]: fabOnReact/react-native-notes#9 (comment)
[4]: fabOnReact/react-native-notes#9 (comment)
[5]: fabOnReact/react-native-notes#9 (comment)
[6]: fabOnReact/react-native-notes#9 (comment)
[7]: fabOnReact/react-native-notes#9 (comment)
[8]: fabOnReact/react-native-notes#9 (comment)

[10]: https://github.com/blavalla "blavalla github profile"
[12]: https://github.com/aosp-mirror/platform_frameworks_base/blob/1ac46f932ef88a8f96d652580d8105e361ffc842/core/java/com/android/internal/widget/ExploreByTouchHelper.java#L48 "com/android/internal/widget/ExploreByTouchHelper.java#L48"
[13]: fabOnReact/react-native-notes#9 (comment) "explanation of getVirtualViewAt and onPopulateBoundsForVirtualView"
[14]: https://github.com/aosp-mirror/platform_frameworks_base/blob/1ac46f932ef88a8f96d652580d8105e361ffc842/core/java/android/text/Spannable.java#L3 "core/java/android/text/Spannable.java#L3"
[15]: https://github.com/fabriziobertoglio1987/react-native/blob/561266fc180b96d6337d6c6c5c3323522d66cc44/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java#L70-L71 "react/views/text/ReactTextViewManager.java#L70-L71"
[16]: https://github.com/fabriziobertoglio1987/react-native/blob/561266fc180b96d6337d6c6c5c3323522d66cc44/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java#L680-L685 "react/uimanager/ReactAccessibilityDelegate.java#L680-L685"
[17]: https://github.com/facebook/react-native/blob/561266fc180b96d6337d6c6c5c3323522d66cc44/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java#L126-L129 "react/views/text/TextLayoutManager.java#L126-L129"
[18]: b352e2d
[19]: #30375 (comment) "explanation on how nested Text are converted to Android Spans"
[20]: https://github.com/aosp-mirror/platform_frameworks_base/blob/1ac46f932ef88a8f96d652580d8105e361ffc842/core/java/android/widget/TextView.java#L214-L220 "core/java/android/widget/TextView.java#L214-L220"
[21]: https://github.com/facebook/react-native/blob/485cf6118b0ab0b59e078b96701b69ae64c4dfb7/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java#L577 "dispatchHoverEvent in ReactTextView"
[22]: https://github.com/aosp-mirror/platform_frameworks_base/blob/1ac46f932ef88a8f96d652580d8105e361ffc842/core/java/com/android/internal/widget/ExploreByTouchHelper.java#L120-L138 "dispatchHoverEvent in ExploreByTouchHelper"
[23]: #32004
[24]: #31757
[25]: https://github.com/fabriziobertoglio1987/react-native/blob/485cf6118b0ab0b59e078b96701b69ae64c4dfb7/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java#L648 "setting link.id in the AccessibilityLink constructor"
[26]: https://github.com/facebook/react-native/pull/33215/files/485cf6118b0ab0b59e078b96701b69ae64c4dfb7#r820014411 "comment on role of link.id"

Reviewed By: blavalla

Differential Revision: D34687371

Pulled By: philIip

fbshipit-source-id: 8e63c70e9318ad8d27317bd68497705e595dea0f
fabOnReact added a commit to fabOnReact/react-native-website that referenced this issue Aug 3, 2022
…ested Text Components with accessibilityRole="link" or inline Images

facebook/react-native#30850 (comment)
<details><summary>Nested Test with accessibilityRole="link" is accessible when parent sets importantForAccessibility="yes"</summary>
<p>

https://github.com/facebook/react-native/pull/33215/files#diff-37199b6b562523b453547e7fb56c95fd9aed5dc01fee3f6bdd50e97297ff8e7fR78
https://developer.android.com/reference/android/view/View#IMPORTANT_FOR_ACCESSIBILITY_YES
facebook/react-native#30375 (comment)

<details><summary>sourcecode of the example</summary>
<p>

```javascript
class AccessibilityExample extends React.Component<{}> {
  render(): React.Node {
    const uri =
      'https://portfoliofabrizio.s3.eu-central-1.amazonaws.com/videos/surfcheck-poster.png';
    return (
      <View>
        <RNTesterBlock title="TextView without label">
          <Text importantForAccessibility="no" accessible={true}>
            This is a Text with an Image as nested Child and a Link.
            <Text
              accessibilityRole="link">
              This is a link
            </Text>
            <Image
              accessible={true}
              focusable={true}
              importantForAccessibility="yes"
              accessibilityLabel="image accessibilityLabel"
              source={{uri}}
              style={{height: 400, width: 200}}
            />
          </Text>
        </RNTesterBlock>
      </View>
    );
  }
}
```

</p>

</details>

<video src="https://user-images.githubusercontent.com/24992535/180412450-071467b1-41ca-4818-b8cc-cbe966b65859.mp4" width="1000" />

</p>
</details>

<details><summary>Nested Test with accessibilityRole="link" is **not** accessible when parent sets importantForAccessibility="no"</summary>
<p>

<video src="https://user-images.githubusercontent.com/24992535/180390384-62129561-5d80-417b-8146-15acee128f76.mp4" width="1000" />

</p>
</details>

<details><summary>ExploreByTouchHelper#requestAccessibiltyFocus</summary>
<p>

https://github.com/aosp-mirror/platform_frameworks_base/blob/19e53cfdc8a5c6ef45c0adf2dd239576ddce5822/core/java/com/android/internal/widget/ExploreByTouchHelper.java#L595

```java
    /**
     * Attempts to give accessibility focus to a virtual view.
     * <p>
     * A virtual view will not actually take focus if
     * {@link AccessibilityManager#isEnabled()} returns false,
     * {@link AccessibilityManager#isTouchExplorationEnabled()} returns false,
     * or the view already has accessibility focus.
     *
     * @param virtualViewId The id of the virtual view on which to place
     *            accessibility focus.
     * @return Whether this virtual view actually took accessibility focus.
     */
    private boolean requestAccessibilityFocus(int virtualViewId) {
        final AccessibilityManager accessibilityManager =
                (AccessibilityManager) mContext.getSystemService(Context.ACCESSIBILITY_SERVICE);

        if (!mManager.isEnabled()
                || !accessibilityManager.isTouchExplorationEnabled()) {
            return false;
        }
        // TODO: Check virtual view visibility.
        if (!isAccessibilityFocused(virtualViewId)) {
            // Clear focus from the previously focused view, if applicable.
            if (mFocusedVirtualViewId != INVALID_ID) {
                sendEventForVirtualView(mFocusedVirtualViewId,
                        AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUS_CLEARED);
            }

            // Set focus on the new view.
            mFocusedVirtualViewId = virtualViewId;

            // TODO: Only invalidate virtual view bounds.
            mView.invalidate();
            sendEventForVirtualView(virtualViewId,
                    AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUSED);
            return true;
        }
        return false;
    }
```

</p>
</details>

<details><summary>View#onFocusChanged</summary>
<p>

https://github.com/aosp-mirror/platform_frameworks_base/blob/19e53cfdc8a5c6ef45c0adf2dd239576ddce5822/core/java/android/view/View.java#L12837

```java
    /**
     * Called by the view system when the focus state of this view changes.
     * When the focus change event is caused by directional navigation, direction
     * and previouslyFocusedRect provide insight into where the focus is coming from.
     * When overriding, be sure to call up through to the super class so that
     * the standard focus handling will occur.
     *
     * @param gainFocus True if the View has focus; false otherwise.
     * @param direction The direction focus has moved when requestFocus()
     *                  is called to give this view focus. Values are
     *                  {@link #FOCUS_UP}, {@link #FOCUS_DOWN}, {@link #FOCUS_LEFT},
     *                  {@link #FOCUS_RIGHT}, {@link #FOCUS_FORWARD}, or {@link #FOCUS_BACKWARD}.
     *                  It may not always apply, in which case use the default.
     * @param previouslyFocusedRect The rectangle, in this view's coordinate
     *        system, of the previously focused view.  If applicable, this will be
     *        passed in as finer grained information about where the focus is coming
     *        from (in addition to direction).  Will be <code>null</code> otherwise.
     */
    @callsuper
    protected void onFocusChanged(boolean gainFocus, @FocusDirection int direction,
            @nullable Rect previouslyFocusedRect) {
        if (gainFocus) {
            sendAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_FOCUSED);
        } else {
            notifyViewAccessibilityStateChangedIfNeeded(
                    AccessibilityEvent.CONTENT_CHANGE_TYPE_UNDEFINED);
        }

        // Here we check whether we still need the default focus highlight, and switch it on/off.
        switchDefaultFocusHighlight();

        if (!gainFocus) {
            if (isPressed()) {
                setPressed(false);
            }
            if (hasWindowFocus()) {
                notifyFocusChangeToImeFocusController(false /* hasFocus */);
            }
            onFocusLost();
        } else if (hasWindowFocus()) {
            notifyFocusChangeToImeFocusController(true /* hasFocus */);
        }

        invalidate(true);
        ListenerInfo li = mListenerInfo;
        if (li != null && li.mOnFocusChangeListener != null) {
            li.mOnFocusChangeListener.onFocusChange(this, gainFocus);
        }

        if (mAttachInfo != null) {
            mAttachInfo.mKeyDispatchState.reset(this);
        }

        if (mParent != null) {
            mParent.onDescendantUnbufferedRequested();
        }

        notifyEnterOrExitForAutoFillIfNeeded(gainFocus);
    }
```
</p>
</details>

<details><summary>View#findAccessibilityFocusHost</summary>
<p>

```java
    /**
     * Returns the view within this view's hierarchy that is hosting
     * accessibility focus.
     *
     * @param searchDescendants whether to search for focus in descendant views
     * @return the view hosting accessibility focus, or {@code null}
     */
    private View findAccessibilityFocusHost(boolean searchDescendants) {
        if (isAccessibilityFocusedViewOrHost()) {
            return this;
        }

        if (searchDescendants) {
            final ViewRootImpl viewRoot = getViewRootImpl();
            if (viewRoot != null) {
                final View focusHost = viewRoot.getAccessibilityFocusedHost();
                if (focusHost != null && ViewRootImpl.isViewDescendantOf(focusHost, this)) {
                    return focusHost;
                }
            }
        }

        return null;
    }
```

</p>
</details>
@fabOnReact fabOnReact self-assigned this Dec 22, 2022
@fabOnReact
Copy link
Contributor

fabOnReact commented Dec 22, 2022

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 react-native-community/discussions-and-proposals#495 (comment)

@fabOnReact
Copy link
Contributor

fabOnReact commented Jan 3, 2023

@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 #35704 (comment)

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

Screenshot 2023-01-03 at 3 33 55 PM

Screenshot 2023-01-03 at 3 37 09 PM

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/

Screenshot 2023-01-03 at 4 06 42 PM

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).

Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this issue Jan 15, 2023
Summary:
This issue fixes [32004][23]. The Pull Request was previously published by [blavalla][10] with [31757][24].
>This is a follow-up on [D23553222 (https://github.com/facebook/react-native/commit/b352e2da8137452f66717cf1cecb2e72abd727d7)][18], which made links functional by using [Talkback's Links menu][1]. We don't often use this as the sole access point for links due to it being more difficult for users to navigate to and easy for users to miss if they don't listen to the full description, including the hint text that announces that links are available.
The Implementation of the functionality consists of:

Retrieving the accessibility links and triggering the TalkBack Focus over the Text
1. nested Text components with accessibilityRole link are saved as [ReactClickableSpan][17] instances in Android native [TextView][20] ([more info][19])
1. If the TextView contains any [ClickableSpans][15] (which are [nested Text][14] components with role link), set a view tag and reset the accessibility delegate.
3. Obtain each link description, start, end, and position relative to the parent Text (id) from the Span as an [AccessibilityLink][16]
4. Use the [AccessibilityLink][16]  to display TalkBack focus over the link with the `getVirtualViewAt` method (more [info][13])

Implementing ExploreByTouchHelper to detect touches over links and to display TalkBack rectangle around them.
1. ReactAccessibilityDelegate inherits from [ExploreByTouchHelper][12]
2. If the [ReactTextView][21] has an accessibility delegate, trigger ExploreByTouchHelper method [dispatchHoverEvent][22]
3.  Implements the methods `getVirtualViewAt` and `onPopulateBoundsForVirtualView`.
     The two methods implements the following functionalities  (more [info][13]):
    * detecting the TalkBack onPress/focus on nested Text with accessibilityRole="link"
    * displaying TalkBack rectangle around nested Text with accessibilityRole="link"

## Changelog

[Android] [Added] - Make links independently focusable by Talkback

Pull Request resolved: facebook#33215

Test Plan:
[1]. User Interacts with links through TalkBack default accessibility menu ([link][1])
[2]. The nested link becomes the next focusable element after the parent element that contains it. ([link][2])
[3]. Testing accessibility examples in pr branch ([link][3])
[4]. Testing accessibility android examples in pr branch ([link][4])
[7]. TalkBack focus moves through links in the correct order from top to bottom (PR Branch with [link.id][25]) ([link to video test][7]) ([discussion][26])
[8]. TalkBack focus does not move through links in the correct order from top to bottom (PR Branch without [link.id][25]) ([link to video test][8]) ([discussion][26])

Test on main branch
[5]. Testing accessibility examples in main branch ([link][5])
[6]. Testing accessibility android examples in main branch ([link][6])

[1]: fabOnReact/react-native-notes#9 (comment)
[2]: fabOnReact/react-native-notes#9 (comment)
[3]: fabOnReact/react-native-notes#9 (comment)
[4]: fabOnReact/react-native-notes#9 (comment)
[5]: fabOnReact/react-native-notes#9 (comment)
[6]: fabOnReact/react-native-notes#9 (comment)
[7]: fabOnReact/react-native-notes#9 (comment)
[8]: fabOnReact/react-native-notes#9 (comment)

[10]: https://github.com/blavalla "blavalla github profile"
[12]: https://github.com/aosp-mirror/platform_frameworks_base/blob/1ac46f932ef88a8f96d652580d8105e361ffc842/core/java/com/android/internal/widget/ExploreByTouchHelper.java#L48 "com/android/internal/widget/ExploreByTouchHelper.java#L48"
[13]: fabOnReact/react-native-notes#9 (comment) "explanation of getVirtualViewAt and onPopulateBoundsForVirtualView"
[14]: https://github.com/aosp-mirror/platform_frameworks_base/blob/1ac46f932ef88a8f96d652580d8105e361ffc842/core/java/android/text/Spannable.java#L3 "core/java/android/text/Spannable.java#L3"
[15]: https://github.com/fabriziobertoglio1987/react-native/blob/561266fc180b96d6337d6c6c5c3323522d66cc44/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextViewManager.java#L70-L71 "react/views/text/ReactTextViewManager.java#L70-L71"
[16]: https://github.com/fabriziobertoglio1987/react-native/blob/561266fc180b96d6337d6c6c5c3323522d66cc44/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java#L680-L685 "react/uimanager/ReactAccessibilityDelegate.java#L680-L685"
[17]: https://github.com/facebook/react-native/blob/561266fc180b96d6337d6c6c5c3323522d66cc44/ReactAndroid/src/main/java/com/facebook/react/views/text/TextLayoutManager.java#L126-L129 "react/views/text/TextLayoutManager.java#L126-L129"
[18]: facebook@b352e2d
[19]: facebook#30375 (comment) "explanation on how nested Text are converted to Android Spans"
[20]: https://github.com/aosp-mirror/platform_frameworks_base/blob/1ac46f932ef88a8f96d652580d8105e361ffc842/core/java/android/widget/TextView.java#L214-L220 "core/java/android/widget/TextView.java#L214-L220"
[21]: https://github.com/facebook/react-native/blob/485cf6118b0ab0b59e078b96701b69ae64c4dfb7/ReactAndroid/src/main/java/com/facebook/react/views/text/ReactTextView.java#L577 "dispatchHoverEvent in ReactTextView"
[22]: https://github.com/aosp-mirror/platform_frameworks_base/blob/1ac46f932ef88a8f96d652580d8105e361ffc842/core/java/com/android/internal/widget/ExploreByTouchHelper.java#L120-L138 "dispatchHoverEvent in ExploreByTouchHelper"
[23]: facebook#32004
[24]: facebook#31757
[25]: https://github.com/fabriziobertoglio1987/react-native/blob/485cf6118b0ab0b59e078b96701b69ae64c4dfb7/ReactAndroid/src/main/java/com/facebook/react/uimanager/ReactAccessibilityDelegate.java#L648 "setting link.id in the AccessibilityLink constructor"
[26]: https://github.com/facebook/react-native/pull/33215/files/485cf6118b0ab0b59e078b96701b69ae64c4dfb7#r820014411 "comment on role of link.id"

Reviewed By: blavalla

Differential Revision: D34687371

Pulled By: philIip

fbshipit-source-id: 8e63c70e9318ad8d27317bd68497705e595dea0f
@github-actions
Copy link

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.

@github-actions github-actions bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Sep 15, 2023
@miallo
Copy link
Contributor

miallo commented Sep 15, 2023

Still an issue

@github-actions github-actions bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Sep 15, 2023
@cortinico cortinico added the Never gets stale Prevent those issues and PRs from getting stale label Sep 15, 2023
@paulschreiber
Copy link
Contributor

Still a problem with 0.72.6.

@fabOnReact
Copy link
Contributor

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
Copy link
Contributor

I'm the author of #35949 and #35704. The two PRs fix this issue.

@fabOnReact fabOnReact removed their assignment Jan 19, 2024
@ngoclamnn
Copy link

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

@qwertychouskie
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs: Triage 🔍 Never gets stale Prevent those issues and PRs from getting stale Platform: Android Android applications. Platform: Linux Building on Linux.
Projects
None yet
10 participants