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

feat: add isAccessibilityServiceEnabled #31396

Closed

Conversation

grgr-dkrk
Copy link
Contributor

Summary

fix #30863

This PR adds isAccessibilityServiceEnabled to get if accessibility services are enabled on Android.

Changelog

[Android] [Added] - Added isAccessibilityServiceEnabled to get if accessibility services are enabled

Test Plan

accessibilityService

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 21, 2021
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:

@@ -46,6 +46,17 @@ public void onTouchExplorationStateChanged(boolean enabled) {
}
}

// Android can listen for accessibility service enable with `accessibilityStateChange`, but `accessibilityState` conflicts with React Native props and confuses developers. Therefore, the name `accessibilityServiceChange` is used here instead.

Choose a reason for hiding this comment

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

google-java-format suggested changes:

@@ -49 +49,3 @@
-  // Android can listen for accessibility service enable with `accessibilityStateChange`, but `accessibilityState` conflicts with React Native props and confuses developers. Therefore, the name `accessibilityServiceChange` is used here instead.
+  // Android can listen for accessibility service enable with `accessibilityStateChange`, but
+  // `accessibilityState` conflicts with React Native props and confuses developers. Therefore, the
+  // name `accessibilityServiceChange` is used here instead.

}
}
}

@Override
@TargetApi(Build.VERSION_CODES.LOLLIPOP)

Choose a reason for hiding this comment

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

google-java-format suggested changes:

@@ -176,2 +178 @@
-    mAccessibilityManager.addAccessibilityStateChangeListener(
-        mAccessibilityServiceChangeListener);
+    mAccessibilityManager.addAccessibilityStateChangeListener(mAccessibilityServiceChangeListener);

@analysis-bot
Copy link

analysis-bot commented Apr 21, 2021

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

Base commit: 61e1b6f
Branch: main

@amarlette
Copy link

Thank you for submitting this @grgr-dkrk! I've added your PR to the Improved React Native Accessibility board. Thank you for your contribution to a more accessible React Native.

@kacieb kacieb self-assigned this Apr 23, 2021
@kacieb kacieb requested a review from blavalla April 23, 2021 19:30
@@ -33,6 +33,7 @@ type AccessibilityEventDefinitions = {
change: [boolean], // screenReaderChanged
reduceMotionChanged: [boolean],
screenReaderChanged: [boolean],
accessibilityServiceChanged: [boolean],
Copy link
Contributor

Choose a reason for hiding this comment

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

If this event is only supported on Android, we should mark that (perhaps by adding a new type AccessibilityEventDefinitionsAndroid like iOS has.

@kacieb
Copy link
Contributor

kacieb commented Apr 23, 2021

This is interesting - so this listens for any change? cc @blavalla I know we discussed this a bit, would the potential use case for this be to listen for changes so as to be able to update the layout (or similar) if, for example, TalkBack was turned on/off? So you'd need to listen for changes, and then check if TalkBack is on with another function call.

@blavalla
Copy link
Contributor

This is interesting - so this listens for any change? cc @blavalla I know we discussed this a bit, would the potential use case for this be to listen for changes so as to be able to update the layout (or similar) if, for example, TalkBack was turned on/off? So you'd need to listen for changes, and then check if TalkBack is on with another function call.

Yeah, thats the general idea. Although we already have a method specifically for talkback with screenReaderChanged. This would be able to detect any service changing, since on Android there can be 3rd party accessibility services, the API google provides doesn't have methods for specific services (talkback, switch access, etc.) like iOS does.

@grgr-dkrk grgr-dkrk requested a review from kacieb May 4, 2021 01:00
@react-native-bot react-native-bot added Platform: Android Android applications. Type: Enhancement A new feature or enhancement of an existing feature. labels Oct 1, 2021
@pull-bot
Copy link

PR build artifact for ccc99be is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@analysis-bot
Copy link

analysis-bot commented Oct 19, 2021

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,311,451 +21,438
android hermes armeabi-v7a 7,875,632 +21,571
android hermes x86 8,678,642 +21,603
android hermes x86_64 8,637,847 +21,710
android jsc arm64-v8a 9,813,274 +21,909
android jsc armeabi-v7a 8,774,264 +22,039
android jsc x86 9,762,315 +22,077
android jsc x86_64 10,363,241 +22,173

Base commit: 61e1b6f
Branch: main

@pull-bot
Copy link

PR build artifact for 1e641c1 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@kacieb kacieb requested a review from lunaleaps October 25, 2021 15:08
@kacieb kacieb removed their assignment Oct 25, 2021
@facebook-github-bot facebook-github-bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 25, 2021
@lunaleaps
Copy link
Contributor

lunaleaps commented Oct 25, 2021

@grgr-dkrk Can we update the title since the API is isAccessibilityServiceEnabled instead of isAccessibilityStateEnabled?

And similarly, could you also create some documentation for this API in react-native-website? Thank you!

@facebook-github-bot
Copy link
Contributor

@lunaleaps has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@grgr-dkrk grgr-dkrk changed the title feat: add isAccessibilityStateEnabled feat: add isAccessibilityServiceEnabled Oct 26, 2021
@grgr-dkrk
Copy link
Contributor Author

@lunaleaps

And similarly, could you also create some documentation for this API in react-native-website?

I'll deal with it as soon as possible. Thanks!

@grgr-dkrk
Copy link
Contributor Author

@lunaleaps
I have sent a PR to the documentation for this feature. Thanks.
facebook/react-native-website#2830

Copy link
Contributor

@lunaleaps lunaleaps left a comment

Choose a reason for hiding this comment

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

Some additional clarification requested.

* Returns a promise which resolves to a boolean.
* The result is `true` when any service is enabled and `false` otherwise.
*
* See https://reactnative.dev/docs/accessibilityinfo.html#isAccessibilityServiceEnabled
Copy link
Contributor

Choose a reason for hiding this comment

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

Point by @philIip that this comment should also mention that it's an Android only feature

@lunaleaps
Copy link
Contributor

Small comments, I can also add if you're okay with the suggested change, otherwise looks good. Just want to get more experienced Android eyes on this as well internally!

@pull-bot
Copy link

PR build artifact for ab66bd2 is ready.
To use, download tarball from "Artifacts" tab in this CircleCI job then run yarn add <path to tarball> in your React Native project.

@facebook-github-bot
Copy link
Contributor

@lunaleaps has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@lunaleaps merged this pull request in c8b83d4.

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Oct 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accessibility CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Needs: React Native Team Attention Platform: Android Android applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. Type: Enhancement A new feature or enhancement of an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android: System Property Detection Detect State change
10 participants