-
Notifications
You must be signed in to change notification settings - Fork 9.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
Co-author revamped: less CodeMirror, more accessibility #16335
Conversation
Co-Authored-By: Pablo Núñez <pablonete@github.com>
* the screen reader to read the content again. This is useful when the content | ||
* is the same but the screen reader should read it again. | ||
*/ | ||
export class AriaLiveContainer extends Component<IAriaLiveContainerProps> { |
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.
Probably interesting to use in other places 😄
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.
Most definitely.. On the commit completion status I am tagging on the commit short sha for this purpose. I could do this instead, tho I guess the short sha does serve the purpose in clearly indicating to the user that is a different commit with the same summary (if they remember what the last short sha was :D)
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.
✨ Great work!
Closes https://github.com/github/accessibility-audits/issues/3287
Closes https://github.com/github/accessibility-audits/issues/3246
Closes https://github.com/github/accessibility-audits/issues/3244
Closes https://github.com/github/accessibility-audits/issues/3014
Description
Up until now, the co-author input field was implemented using CodeMirror under the hood for reasons (probably because of how nice it is to have a styled input text). However, due to CodeMirror, it's hard to make the component accessible.
This PR rewrites that component from the scratch, uses the existing
AutocompletingTextInput
component (that was made accessible in #16324), and adds other accessibility improvements needed:Co-authors
labelIt also changes a bit the UX:
One feature that has been lost (for now, at least) is undoing/redoing. Not sure how critical that is, but didn't want to make this PR bigger.
Summary of Code Changes
IAuthor
interface with the newAuthor
type, that is the sum ofKnownAuthor
(for users we know exist in GH) andUnknownAuthor
(for users that are either being searched for, or not found). This is what made this PR affect so many files, but it's not that bad!CoAuthorAutocompletionProvider
. It reuses most of the code from ourUserAutocompletionProvider
(used for user autocompletion when you type@
in the commit message textboxes) and only adds support for "unknown users" (AKA random strings that we check if match existing GH users). It also removes the requirement of@
at the beginning, allowing us to always show autocomplete suggestions.AutocompletingInput
:alwaysAutocomplete
: every time the input has focus, it will show the suggestions.autocompleteItemFilter
prop so we can filter out users that are already addedCommitMessage
now checks if there are unknown authors and shows a confirmation dialog warning about that (theUnknownAuthors
dialog). Slightly hacky, but I madeCommitMessage
to passthis.createCommit
as theonCommitAnyway
callback of the said dialog.AuthorInput
andAuthorHandle
are all new, sorry.AriaLiveContainer
as an easy & reusable way to force screen readers to re-read aria live sections. In this example: if you type something and get back exactly the same number of suggestions as before, this component will add/remove a
to force the screen reader to read it.Screenshots
CleanShot.2023-03-24.at.12.09.17.mp4
Release notes
Notes: [Improved] Make co-authors input text accessible