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

iOS[accessibility]: Added accessibility split focus prop for iOS #44117

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

Biki-das
Copy link
Contributor

@Biki-das Biki-das commented Apr 16, 2024

Summary:

Currently in iOS we have no way to access nested elements in a View through accessibility methods such has voice over
like if we have hierarchy like below

<Parent Element>
 <Child1/>
<Child2/>
<Parent Element/>

We cannot focus on the individual child elements and get accessibility details which works on android, to allow for the same, i have tried to supply an additional Prop limited to iOS called accessibilitySplitFocus which when set to true will allow for the child elements to be accessible further.

nested.accessibility.prop.mp4

Using the mac accessibility inspector i have tested whether it works or not

Changelog:

[iOS] [Added] - iOS[accessibility]: Added accessibility split focus prop for iOS

Test Plan:

Added a new Example which demonstrates the same, this would require either testing in local device using voice over turned or using the mac inbuilt accessibility inspector.

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Apr 16, 2024
@Biki-das
Copy link
Contributor Author

Biki-das commented Apr 16, 2024

cc @hoxyq @cipolleschi since this is an iOS related change, if this is something we are looking forward to include, will update the documentation of the react native a11y section and add it there as well!

@analysis-bot
Copy link

analysis-bot commented Apr 16, 2024

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 19,478,524 +124
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 22,851,058 +51
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 237ddb4
Branch: main

@Biki-das
Copy link
Contributor Author

Biki-das commented Apr 16, 2024

need some help with the snapshot 😅

@blakef blakef added Platform: iOS iOS applications. Accessibility labels Apr 18, 2024
@Biki-das
Copy link
Contributor Author

@blakef could you possibly review this!
further tagging @hoxyq , to connect this with someone who could review this.

@facebook-github-bot
Copy link
Contributor

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

@Biki-das
Copy link
Contributor Author

@blakef could you help me know what failed since this looks like only accessible to only meta members!

@Biki-das
Copy link
Contributor Author

@hoxyq need some help on the fb internal test fail could you help!

@Biki-das
Copy link
Contributor Author

friendly ping @blakef

@blakef
Copy link
Contributor

blakef commented May 1, 2024

Hi @Biki-das, thanks for the PR. We'll get back to you on this when we're in a position to give useful feedback (so no need to ping). I'm gathering more feedback (always hard to get for accessibility work) on whether this is the best approach.

One bit of feedback we've already had is that the bar is very high to add additional API. You can make my life a little easier for me here. Can you motivate why we'd need the accessibilitySplitFocus prop? There's always a leaning toward doing what web does. Why should this be configurable (through the prop) instead of always on?

@Biki-das
Copy link
Contributor Author

Biki-das commented May 2, 2024

thanks @blakef for the detailed feedback and sorry for the pings, i understand adding a new API and its maintenance is additional work, the convincing point for this Prop exactly in my opinion is, this Prop would bring some consistency with android where accessing nested view is easy you just need to pass the accessible prop and you can access nested child views!
This PR aims to make accessibility focus acts as consistent with that on Android for these nested views.
When touch on the child elements, the corresponding child element would be activated other than the parent, if its accessibilitySplitFocus prop is set to true. Otherwise, you would get a merged focus.

 ---------------------
|   Parent Element <--|---focus 1
|                     |
|  --------------- 
| | Child Element |<--|---focus 2
|  ---------------    |
|  ---------------    |
| | Child Element |<--|---focus 3
|  ---------------    |
|                     |
 ---------------------

i hope the above just clears the usage of this Prop, coming to the second question why not make it the default behaviour and not an additional prop, In my opinion, the case for looking inside child Views should not always be the default case, Developers might just want to skim through all the parent views and if only they feel they might want to investigate the child views, and just inspect the nested ones. i would consider this as a specific case rather than a general and that is why, engineers who worked on react native did not made the same way. ofcourse would love to know other core contributors/maintainers thoughts on the same.
overall i feel this can be a good addition to the iOS a11y prop list, rest i respect the decision of all the concerned, i am happy being part of the community and sharing my ideas.

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. Platform: iOS iOS applications. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants