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

Make the PR Badge into a button component #18268

Merged
merged 1 commit into from Mar 8, 2024

Conversation

tidy-dev
Copy link
Contributor

@tidy-dev tidy-dev commented Mar 7, 2024

xref https://github.com/github/accessibility-audits/issues/6878

Description

This PR updates the PR-badge control to be a button with appropriate aria-has-popup and aria-expanded semantics since it is responsible for opening a popover.

Screenshots

CleanShot.2024-03-07.at.12.10.27.mp4

Release notes

Notes: [Fixed] The pull request check runs button is keyboard accessible.

@tidy-dev tidy-dev changed the title Make the pr-badge into a button component Make the PR Badge into a button component Mar 7, 2024
@tidy-dev tidy-dev requested a review from jacortinas March 7, 2024 17:13
@@ -14,7 +15,10 @@ interface IPullRequestBadgeProps {
/** The GitHub repository to use when looking up commit status. */
readonly repository: GitHubRepository

readonly onBadgeRef?: (ref: HTMLDivElement | null) => void
/** Whether or not the check runs popover is open */
readonly showCIStatusPopover?: boolean
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This attribute is optional because this component is also used in our pull request quick view component and it is not used as a button there. Since the pull request quick view component is feature flagged to development and work is currently stopped on it, I did not think it was worth the time handling the "non-button" view of this component. This can be reworked if that work is picked up again.

@@ -80,16 +82,22 @@ export class PullRequestBadge extends React.Component<
public render() {
const ref = getPullRequestCommitRef(this.props.number)
return (
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
Copy link
Contributor Author

@tidy-dev tidy-dev Mar 7, 2024

Choose a reason for hiding this comment

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

Yay! Getting rid of three a11y linter disables!

@tidy-dev tidy-dev merged commit f97aced into development Mar 8, 2024
7 checks passed
@tidy-dev tidy-dev deleted the pull-request-checks-keyboard-accessible branch March 8, 2024 12:42
@niik niik mentioned this pull request Apr 25, 2024
@tidy-dev tidy-dev added the accessibility Issues related to accessibility improvements label May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accessibility Issues related to accessibility improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants