Ventura dialog semantics workaround #17148
Conversation
| () => | ||
| __DARWIN__ && | ||
| systemVersionGreaterThanOrEqualTo('13.0') && | ||
| systemVersionLessThan('14.0') |
There was a problem hiding this comment.
I could have just said >= 13.. but I am kind of hoping with sadistic optimism that Apple will correct this in a later version and we can use correct semantics again.. but if they don't, another bug report may arise leading to this needing to be extended to include later versions.
| * `aria-labelledby` where both ids are announced. | ||
| * | ||
| */ | ||
| private getAriaAttributes() { |
There was a problem hiding this comment.
Another option as seen in #17134 would be to set the aria-label attribute to the contents of the props.title. This would be better semantically, but, sometimes that title is not a string and therefore using aria-describedby allows us to still point to the id of the title for both strings and JSX.elements. Additionally, the below has less hairy code.
|
|
||
| if (this.props.role === 'alertdialog') { | ||
| return { | ||
| 'aria-labelledby': `${this.state.titleId} ${this.props.ariaDescribedBy}`, |
There was a problem hiding this comment.
Don't we need to ?? '' the this.props.ariaDescribedBy as in line 760? 🤔
There was a problem hiding this comment.
The props of the dialog are such that if the role=alertdialog than the ariaDescribedBy prop is required.. It should always be populated in this case. Tho, I think typescript probably sees it as possibly undefined, and also if somehow this was provided as undefined (like at runtime), the worst case is it prints "undefined" which just doesn't do anything.
There was a problem hiding this comment.
The props of the dialog are such that if the
role=alertdialogthan theariaDescribedByprop is required.. It should always be populated in this case. Tho, I think typescript probably sees it as possibly undefined, and also if somehow this was provided as undefined (like at runtime), the worst case is it prints "undefined" which just doesn't do anything.
xref: https://github.com/github/accessibility-audits/issues/4979
Description
This PR is targeted workaround for macOS Ventura which introduced a regression in announcing dialog titles and descriptions.
For role of
dialog(default), thearia-labelledbyis not announced and if provided prevents thearia-describedbyfrom being announced. Thus, in this case, we will add thearia-labelledbyto thearia-describedby.For role of
alertdialog, thearia-labelledbyis announced but not thearia-describedby. Thus, in this case, we will add both to thearia-labelledbyFor windows, we maintain correct semantics in that the dialog element should have
aria-labelledbyand thearia-describedbyis optional unless the dialog has a role ofalertdialog, in which case both are required.A known caveat:
aria-describedbyas "{aria-labelledby-id aria-describedby-id}". macOS only reads the first one and mentions the existence of "other things". It is unknown whether that was expected behavior before the regression. We do not currently have any non alert dialogs with a description and therefore this does not impact us.Screenshots
macOS Ventura
CleanShot.2023-07-31.at.08.20.46.mp4
windows
CleanShot.2023-07-31.at.08.16.18.mp4
Release notes
Notes: [Fixed] Adds a workaround for the macOS Ventura
aria-labelledbyandaria-describedbyregressions such that dialog titles are always announced.