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

Co-author revamped: less CodeMirror, more accessibility #16335

Merged
merged 56 commits into from
Mar 29, 2023

Conversation

sergiou87
Copy link
Member

@sergiou87 sergiou87 commented Mar 17, 2023

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:

  • Screen readers now announce the autocomplete suggestion chosen by the user.
  • Screen readers now announce the Co-authors label

It also changes a bit the UX:

  • Now added co-authors are not part of the input text, but are siblings to it. Users can use arrow keys to navigate through them and delete them with Backspace/Delete.
  • Now the input field always shows autocompletion suggestions when it's focused, since only known users should be added.

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

  • Replaced the IAuthor interface with the new Author type, that is the sum of KnownAuthor (for users we know exist in GH) and UnknownAuthor (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!
  • Added CoAuthorAutocompletionProvider. It reuses most of the code from our UserAutocompletionProvider (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.
  • Some new features to our AutocompletingInput:
    • Added support for alwaysAutocomplete: every time the input has focus, it will show the suggestions.
    • Added autocompleteItemFilter prop so we can filter out users that are already added
  • CommitMessage now checks if there are unknown authors and shows a confirmation dialog warning about that (the UnknownAuthors dialog). Slightly hacky, but I made CommitMessage to pass this.createCommit as the onCommitAnyway callback of the said dialog.
  • AuthorInput and AuthorHandle are all new, sorry.
  • Added 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

@sergiou87 sergiou87 marked this pull request as ready for review March 24, 2023 11:12
@sergiou87 sergiou87 marked this pull request as draft March 24, 2023 12:07
@sergiou87 sergiou87 marked this pull request as ready for review March 27, 2023 10:44
* 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> {
Copy link
Member Author

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 😄

Copy link
Contributor

@tidy-dev tidy-dev Mar 29, 2023

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)

Copy link
Contributor

@tidy-dev tidy-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✨ Great work!

@sergiou87 sergiou87 merged commit 56ae463 into development Mar 29, 2023
@sergiou87 sergiou87 deleted the co-author-accessibility-fixes branch March 29, 2023 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants