Dialog headers needing announced. #17631
Conversation
This adds a workaround for macOS Sonoma..
| if (!isMacOSVentura()) { | ||
| if (isMacOSVentura()) { |
There was a problem hiding this comment.
Decided to invert the checks since now we have multiple. Basically the bottom return statement is the "expected" one if we do not have any if that redirect it.
| } | ||
| } | ||
|
|
||
| if (this.props.role === 'alertdialog') { | ||
| if (isMacOSSonoma() && this.props.role !== 'alertdialog') { |
There was a problem hiding this comment.
macSomona did improved that their alertdialogs work as intended where Ventura didn't! :)
| systemVersionGreaterThanOrEqualTo('14.0') && | ||
| systemVersionLessThan('15.0') |
There was a problem hiding this comment.
Sadly, this either means version 15 will need a similar addition or they will fix it in 15.. They did make some improvements from 13 to 14. 🤞 Feedback from accessibility team is that we are acknowledging that this may not be readily caught until another audit.
| <span id="popover-dropdown-header">{contentTitle}</span> | ||
| <h3 id="popover-dropdown-header">{contentTitle}</h3> |
There was a problem hiding this comment.
Header should be a header anyways.. but is important for macOS VoiceOver
| <h3 id="diff-options-popover-header"> | ||
| Diff {__DARWIN__ ? 'Settings' : 'Options'} | ||
| </h3> | ||
| <h3 id="diff-options-popover-header">{header}</h3> |
There was a problem hiding this comment.
This isn't necessary... NVDA nicely announces it as "Diff Options" but VoiceOver was doing "Diff level 2 settings". I assume because the {__DARWIN__ ? 'Settings' : 'Options'} was adding in a detectable child element.. This just make the VoiceOver announcement a clean Diff Settings
xref: https://github.com/github/accessibility-audits/issues/6728
Description
Seeing that the prop for the
ariaLabelledbyin thepopover-dropdownwasaria-labelledby. I figured getting the branch select dropdown header to announce would be a easy fix. However, in doing so, I found that it would announce for NVDA but it would not announce for VoiceOver. 😠 Did some spot checking and most of our dialogs no longer announce their titles. 😞So.. easy fix turned into figuring out the new VoiceOver problem. Turns out that VoiceOver works fine if an dialog element had a header child that is NOT associated via the
aria-labelledby. This is incorrect as it should work with the semantics wired up. But, VoiceOver wants to do it's auto detection thing.Technically, we could leave it correctly wired and just file a bug with Apple. But, that leaves all our VoiceOver users in a lurch. Thus, in this PR, I added
isMacOSSonomacheck to our dialog and popover dialogs similar to ourisMacOSVentura, fortunately Apple improved upon the dialog title announcing behavior of Sonoma so it is less complex. That said, in the next major version of macOS, we may end up needing to do this again.. It is a bummer, but is the best thing to do for our users.Screenshots
Tested our popovers, regular dialogs, and confirm (alert) dialogs.
CleanShot.2023-10-26.at.11.22.47.mp4
Release notes
Notes:
[Fixed] The branch select popover header in the pull request preview dialog is announced by screen readers.
[Fixed] The dialog and popover headers are announced by VoiceOver in macOs Sonoma.