Skip to content

Adds AccessibilityInfo.isScreenReaderEnabled to RN MacOS#444

Merged
tom-un merged 17 commits intomicrosoft:masterfrom
graytmatterMS:matgray/a11yInfo-voiceOver-enable
Jun 11, 2020
Merged

Adds AccessibilityInfo.isScreenReaderEnabled to RN MacOS#444
tom-un merged 17 commits intomicrosoft:masterfrom
graytmatterMS:matgray/a11yInfo-voiceOver-enable

Conversation

@graytmatterMS
Copy link
Copy Markdown

@graytmatterMS graytmatterMS commented Jun 10, 2020

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

When the MacOS fork was made all the AccessibilityInfo functionality was stubbed out because the iOS Accessibility APIs are different from the MacOS APIs. I've filled back out the functionality for isScreenReaderEnabled, and the corresponding event screenReaderChanged.

Previously the Native side implementation of the AccessibilityManager was surrounded by a big ifdef to make sure it was only actually implemented for the supported iOS platform. I've now forked the file so it has a MacOS version. This currently only exports the one method and the one event-listener, but that will change as more features are requested.

Changelog

[MacOS] [Added] - Implemented AccessibilityInfo.isScreenReaderEnabled and screenReaderChanged

Test Plan

  1. Run RNTester
  2. Go to the accessibility page
  3. Scroll to screen reader enabled (second to last)
  4. Toggle VoiceOver in settings.
  • You should see the text change back and forth between "The screen reader is disabled" and "The screen reader is enabled"

Screen Shot 2020-06-10 at 12 27 29
Screen Shot 2020-06-10 at 12 27 37

Microsoft Reviewers: Open in CodeFlow

@graytmatterMS graytmatterMS requested a review from tom-un as a code owner June 10, 2020 19:29
@ghost
Copy link
Copy Markdown

ghost commented Jun 10, 2020

CLA assistant check
All CLA requirements met.

Copy link
Copy Markdown

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

@tom-un
Copy link
Copy Markdown
Collaborator

tom-un commented Jun 10, 2020

There are some Flow errors being reported in the CI. Locally, run yarn flow-check-macos to diagnose. You can also run yarn flow-check-ios and yarn flow-check-android

Copy link
Copy Markdown
Collaborator

@tom-un tom-un left a comment

Choose a reason for hiding this comment

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

Thanks! Made some minor change requests and there's a couple things you'll need to change to pass CI.

@tom-un
Copy link
Copy Markdown
Collaborator

tom-un commented Jun 10, 2020

Oh, bonus: now that we're not compiling the iOS version of RCTAccessilibytManager.m please remove the // TODO(macOS ISS#2323203) lines from the iOS file.

@ghost ghost removed the Needs: Author Feedback label Jun 10, 2020
Co-authored-by: Tommy Nguyen <4123478+tido64@users.noreply.github.com>
@@ -56,7 +69,13 @@ const AccessibilityInfo = {
* Android and iOS only
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This comment now needs to be updated.
You could copy the comment from the .ios.js or .android.js version.

@tom-un tom-un merged commit 1e5eabd into microsoft:master Jun 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants