-
Notifications
You must be signed in to change notification settings - Fork 24.4k
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
iOS: Introduce API for making screen reader announcements #14168
Conversation
VOICE_OVER_EVENT, | ||
handler | ||
); | ||
var listener; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no-trailing-spaces: Trailing spaces not allowed.
handler | ||
); | ||
} | ||
|
||
_subscriptions.set(handler, listener); | ||
return { | ||
remove: AccessibilityInfo.removeEventListener.bind(null, eventName, handler), | ||
}; | ||
}, | ||
|
||
/** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no-trailing-spaces: Trailing spaces not allowed.
var listener = RCTDeviceEventEmitter.addListener( | ||
VOICE_OVER_EVENT, | ||
handler | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no-trailing-spaces: Trailing spaces not allowed.
{ | ||
NSDictionary *userInfo = notification.userInfo; | ||
// Response dictionary to populate the event with. | ||
NSDictionary *response = @{@"announcement": userInfo[@"UIAccessibilityAnnouncementKeyStringValue"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the variable names for this instead of the string (so userInfo[UIAccessibilityAnnouncementKeyStringValue]
@javache Thanks for reviewing. I addressed your comment. |
@javache has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
I tried to merge this pull request into the Facebook internal repo but some checks failed. To unblock yourself please check the following: Does this pull request pass all open source tests on GitHub? If not please fix those. Does the code still apply cleanly on top of GitHub master? If not can please rebase. In all other cases this means some internal test failed, for example a part of a fb app won't work with this pull request. I've added the Import Failed label to this pull request so it is easy for someone at fb to find the pull request and check what failed. If you don't see anyone comment in a few days feel free to comment mentioning one of the core contributors to the project so they get a notification. |
This change introduces some APIs that are useful for making announcements through the screen reader on iOS: - `announceForAccessibility`: The screen reader announces the string that is passed in. - `announcementFinished`: An event that fires when the screen reader has finished making an announcement. You can already solve similar problems with RN Android using the `accessibilityLiveRegion` prop. Live regions are a different feature but they can be used to solve the same problem. This commit does not attempt to add live region support in RN iOS because Apple did not build live region support into iOS.
f877700
to
e0900d4
Compare
@javache Thanks for importing. It looks like the import failed due to merge conflicts. I fixed them. |
@javache has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
@rigdern That's awesome! Can we have it cross-platform? So, can we implement |
@shergin Yes, we could add an Android implementation of I'm not sure if there's a way to implement the |
@"success": userInfo[UIAccessibilityAnnouncementKeyWasSuccessful]}; | ||
|
||
#pragma clang diagnostic push | ||
#pragma clang diagnostic ignored "-Wdeprecated-declarations" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this is deprecated. We have to use typed event objects instead of strings.
@@ -101,6 +106,20 @@ - (void)didReceiveNewVoiceOverStatus:(__unused NSNotification *)notification | |||
} | |||
} | |||
|
|||
- (void)accessibilityAnnouncementDidFinish:(__unused NSNotification *)notification |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove __unused
, please.
*/ | ||
announceForAccessibility: function( | ||
announcement: string | ||
): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't need : void
here because it can be inferred.
@shergin The PR was already merged. |
); | ||
var listener; | ||
|
||
if (eventName === 'change') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have to unify native event names with JS ones, this will drastically simplify this code fragment.
@rigdern Oh, I see. Anyways, we can improve this. 😄 |
@shergin They are good suggestions but I won't have time to submit another PR to address them. For future reference, can you share an example of code that uses typed event objects instead of strings? |
@rigdern Yes, sure, I will fix it myself. |
I am doing translation of this document. Can I interpret the 'announce' here as 'broadcast'?The meaning between the two words is different in Chinese. 'Announce' is more about expressing the meaning of the function declaration in coding,'Broadcast' is more about the screen reader. Which one is the right meaning? |
@caocao-melon Am I correct in assuming you a referring to these snippets?
If so, it sounds like "broadcast" is what you want. All of these usages of "announce" have to do with the screen reader speaking some information to the user. |
This change introduces some APIs that are useful for making announcements through the screen reader on iOS:
announceForAccessibility
: The screen reader announces the string that is passed in.announcementFinished
: An event that fires when the screen reader has finished making an announcement.You can already solve similar problems with RN Android using the
accessibilityLiveRegion
prop. Live regions are a different feature but they can be used to solve the same problem. This commit does not attempt to add live region support in RN iOS because Apple did not build live region support into iOS.Test Plan (required)
Verified that
announceForAccessibility
causes VoiceOver to announce the string when VoiceOver is enabled. Verified thatannouncementFinished
fires with the appropriate data in the event object. Additionally, my team has been using this change in our app.Adam Comella
Microsoft Corp.