search

microsoft / react-native-macos

A framework for building native macOS apps with React.
https://microsoft.github.io/react-native-windows/
MIT License
3.49k stars 135 forks source link

[0.73] Fix new role prop after JS-Shim was removed by Meta #2100

Closed FalseLobster closed 6 months ago

FalseLobster commented 6 months ago

Summary:

Prior to 0.73, the new role prop was remapped to accessibilityRole on the JS-side. Starting with #37304, the work needs to be completed on the native side. Since the new prop is ARIA inspired, the mappings are taking from the ARIA Core AAM which disagrees with some of the mappings used in the old accessibilityRole prop. Users of the old prop are unaffected, but the new prop will take the mappings from the spec.

Test Plan:

Tested a variety of permutations of accessibilityRole and role to confirm the behavior looks correct in Accessibility Inspector

Saadnajmi commented 6 months ago

An aside, we'll want to eventually also implement for Fabric, but not blocking for 0.73

Saadnajmi commented 6 months ago

Q: Would it make sense for our main branch version of the PR to use the aria mappings for the accessibilityRole slot, rather than the legacy mappings we defined? I I can imagine we keep the legacy mappings for one version (0.73) and disable it in the next (0.74). Alternatively, maybe put it behind a Feature flag via this JS file or this native file ?

FalseLobster commented 6 months ago

Q: Would it make sense for our main branch version of the PR to use the aria mappings for the accessibilityRole slot, rather than the legacy mappings we defined? I I can imagine we keep the legacy mappings for one version (0.73) and disable it in the next (0.74). Alternatively, maybe put it behind a Feature flag via this JS file or this native file ?

Maybe, I'm open to it. I kinda suspect at some point Meta will drop accessibilityRole like they did accessibilityTraits (even though our impl still has accessibilityTraits). I think we run the risk internally of breaking partners who are using accessibilityRole if we silently change some mappings for them, whereas only applying the new mappings to the new values to the new prop feels safe, which is worth considering, but I suppose that's why you're calling out the feature flag :)