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

Make links independently focusable by Talkback #31757

Closed
wants to merge 1 commit into from

Conversation

blavalla
Copy link
Contributor

Summary:
This follows up on D23553222 (b352e2d), which made links functional by using Talkback's Links menu. 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 entire description, including the hint text that announces that links are available.

Instead, we generally allow links to be focusable after the main text that contains them is focused. This diff adds this functionality for both Paper and Fabric, and also retains the existing Links menu functionality as well, for users who prefer to use it.

Reviewed By: yungsters

Differential Revision: D28691177

@facebook-github-bot facebook-github-bot added p: Facebook Partner: Facebook Partner CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Jun 21, 2021
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D28691177

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed test');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed Inline Links');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed another link');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed long link');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

@pull-bot
Copy link

pull-bot commented Jun 21, 2021

Messages
📖

📋 Missing Changelog - Can you add a Changelog? To do so, add a "## Changelog" section to your PR description. A changelog entry has the following format: [CATEGORY] [TYPE] - Message.

CATEGORY may be:
  • General
  • iOS
  • Android
  • JavaScript
  • Internal (for changes that do not need to be called out in the release notes)

TYPE may be:

  • Added, for new features.
  • Changed, for changes in existing functionality.
  • Deprecated, for soon-to-be removed features.
  • Removed, for now removed features.
  • Fixed, for any bug fixes.
  • Security, in case of vulnerabilities.

MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

📖 📋 Missing Test Plan - Can you add a Test Plan? To do so, add a "## Test Plan" section to your PR description. A Test Plan lets us know how these changes were tested.
📖 📋 Missing Summary - Can you add a Summary? To do so, add a "## Summary" section to your PR description. This is a good place to explain the motivation for making this change.

Generated by 🚫 dangerJS against 0e06e4b

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D28691177

blavalla added a commit to blavalla/react-native that referenced this pull request Jun 21, 2021
Summary:
Pull Request resolved: facebook#31757

This follows up on D23553222 (facebook@b352e2d), which made links functional by using Talkback's Links menu.  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 entire description, including the hint text that announces that links are available.

Instead, we generally allow links to be focusable after the main text that contains them is focused. This diff adds this functionality for both Paper and Fabric, and also retains the existing Links menu functionality as well, for users who prefer to use it.

Reviewed By: yungsters

Differential Revision: D28691177

fbshipit-source-id: 026a7ed63c5431ca70431ee486d42968175cf09b
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed test');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed Inline Links');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed another link');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed long link');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

@analysis-bot
Copy link

analysis-bot commented Jun 21, 2021

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 1e6fddb

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D28691177

blavalla added a commit to blavalla/react-native that referenced this pull request Jun 21, 2021
Summary:
Pull Request resolved: facebook#31757

This follows up on D23553222 (facebook@b352e2d), which made links functional by using Talkback's Links menu.  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 entire description, including the hint text that announces that links are available.

Instead, we generally allow links to be focusable after the main text that contains them is focused. This diff adds this functionality for both Paper and Fabric, and also retains the existing Links menu functionality as well, for users who prefer to use it.

Reviewed By: yungsters

Differential Revision: D28691177

fbshipit-source-id: bd8bab77b5f0a58f1dd7ed309e3200725c4ab750
Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed test');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed Inline Links');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed another link');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed long link');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

Summary:
Pull Request resolved: facebook#31757

This follows up on D23553222 (facebook@b352e2d), which made links functional by using Talkback's Links menu.  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 entire description, including the hint text that announces that links are available.

Instead, we generally allow links to be focusable after the main text that contains them is focused. This diff adds this functionality for both Paper and Fabric, and also retains the existing Links menu functionality as well, for users who prefer to use it.

Reviewed By: yungsters

Differential Revision: D28691177

fbshipit-source-id: 55e675cf84df5dda3b15d81a7f9bf1f071006bd2
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D28691177

Copy link

@analysis-bot analysis-bot left a comment

Choose a reason for hiding this comment

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

Code analysis results:

  • eslint found some issues. Run yarn lint --fix to automatically fix problems.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed test');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed Inline Links');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed another link');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

style={styles.link}
accessibilityRole="link"
onPress={() => {
alert('pressed long link');

Choose a reason for hiding this comment

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

no-alert: Unexpected alert.

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 9,166,969 +43,103
android hermes armeabi-v7a 8,692,952 +43,055
android hermes x86 9,606,316 +43,098
android hermes x86_64 9,572,492 +43,097
android jsc arm64-v8a 10,806,053 +40,891
android jsc armeabi-v7a 9,723,313 +40,865
android jsc x86 10,840,818 +40,913
android jsc x86_64 11,448,484 +40,902

Base commit: 1e6fddb

facebook-github-bot pushed a commit that referenced this pull request 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
Saadnajmi pushed a commit to Saadnajmi/react-native-macos that referenced this pull request 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
@cortinico
Copy link
Contributor

Close as abandoned internally

@cortinico cortinico closed this Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Facebook Partner: Facebook Partner
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants