Skip to content

Fixed Images not announcing in VoiceOver#475

Merged
tom-un merged 15 commits intomicrosoft:masterfrom
graytmatterMS:matgray/a11y-image-fix
Jul 14, 2020
Merged

Fixed Images not announcing in VoiceOver#475
tom-un merged 15 commits intomicrosoft:masterfrom
graytmatterMS:matgray/a11y-image-fix

Conversation

@graytmatterMS
Copy link
Copy Markdown

@graytmatterMS graytmatterMS commented Jun 26, 2020 •
edited
Loading

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

Images were not being announced as Images by Voiceover. This is unexpected behavior and would have caused visually impaired users to miss images in our experience.

Changelog

[MacOS] [Fixed] - Fixed Voiceover reading of Image

Test Plan

  1. Go into RNTester/js/examples/Image/ImageExample.js and add accessible={true} accessibilityLabel="foo" to 'Plain Network Image' on line 415.
  2. Compile and run app.
  3. Navigate to Image page
  4. Turn on Voiceover.
  5. Navigate to the Plain Network Image.
  • You should hear/see voiceover announce "foo, image"

Screen Shot 2020-06-26 at 14 34 36

Microsoft Reviewers: Open in CodeFlow

@graytmatterMS graytmatterMS requested a review from tom-un as a code owner June 26, 2020 21:38
@graytmatterMS graytmatterMS changed the title Matgray/a11y image fix Fixed Images not announcing in VoiceOver Jun 29, 2020
tom-un
tom-un reviewed Jun 29, 2020
View reviewed changes
tom-un
tom-un reviewed Jun 29, 2020
View reviewed changes
tom-un
tom-un requested changes Jun 29, 2020
View reviewed changes
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.

Need to update the type of reactAccessiblityElement in UIView+React.h too.

@tom-un tom-un linked an issue Jul 1, 2020 that may be closed by this pull request
<Image/> on Mac is not accessible unless accessible is false #428
Closed
@ghost ghost removed the Needs: Author Feedback label Jul 2, 2020
tom-un
tom-un reviewed Jul 2, 2020
View reviewed changes
tom-un
tom-un requested changes Jul 2, 2020
View reviewed changes
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.

In the header, the property should be id not id *

@ghost ghost removed the Needs: Author Feedback label Jul 2, 2020
@tom-un
Copy link
Copy Markdown
Collaborator

tom-un commented Jul 14, 2020 •
edited
Loading

As we discussed offline @graytmatterMS I can take over this PR (you can grant me permission to push directly to your branch, or I can make my own).

Changing the returning type to id breaks the iOS compilation :(. I suggested offline that we change from id to UIAccessibliy * and NSAccessibliy * as a type for the property, but that has its own set of issues: both of those are "informal protocols" and there are other compilation breaks when properties are dereferenced. So, I think the best approach is to just keep the property type as-is, and cast the return in the new override in RCTImageView.m

@graytmatterMS
Copy link
Copy Markdown
Author

graytmatterMS commented Jul 14, 2020

As we discussed offline @graytmatterMS I can take over this PR (you can grant me permission to push directly to your branch, or I can make my own).

Changing the returning type to id breaks the iOS compilation :(. I suggested offline that we change from id to UIAccessibliy * and NSAccessibliy * as a type for the property, but that has its own set of issues: both of those are "informal protocols" and there are other compilation breaks when properties are dereferenced. So, I think the best approach is to just keep the property type as-is, and cast the return in the new override in RCTImageView.m

I've added you as a collaborator to my repository. :)

@tom-un tom-un merged commit 9fe70d5 into microsoft:master Jul 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tom-un tom-un tom-un approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

<Image/> on Mac is not accessible unless accessible is false

2 participants

@graytmatterMS @tom-un