Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

[SG-33093] Enable accessibilityAudit on client/web/src/integration/batches.test.ts #33332

Merged
merged 11 commits into from
Apr 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client/shared/src/commandPalette/CommandList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,7 @@ export const CommandListPopoverButton: React.FunctionComponent<CommandListPopove
onClick={toggleIsOpen}
className={classNames(styles.popoverButton, buttonClassName, isOpen && buttonOpenClassName)}
variant={variant}
aria-label="Command list"
>
<Icon as={ConsoleIcon} size="md" />

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
exports[`ActivationDropdown renders the activation dropdown 1`] = `
<DocumentFragment>
<button
aria-controls="menu--2"
aria-controls=""
aria-haspopup="true"
class="bg-transparent align-items-center test-activation-nav-item-toggle activationDropdownButton"
data-reach-menu-button=""
id="menu-button--menu"
id="menu-button-2"
type="button"
>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ export const AppRouterContainer: React.FunctionComponent<AppRouterContainerProps
}) => (
<ElementScroller scrollKey="app-router-container">
{/*
Data Layout data attribute is used to get access to the main layout scroll element (the div element below).
- Data Layout data attribute is used to get access to the main layout scroll element (the div element below).
from child levels in order to handle or react on this container scroll or other important events.
*/}
<div data-layout={true} className={classNames(styles.appRouterContainer, className)} {...rest}>
<main data-layout={true} className={classNames(styles.appRouterContainer, className)} {...rest}>
{children}
</div>
</main>
</ElementScroller>
)
7 changes: 6 additions & 1 deletion client/web/src/enterprise/batches/list/GettingStarted.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,12 @@ export const GettingStarted: React.FunctionComponent<GettingStartedProps> = ({ f
View the product marketing page for a high-level overview of benefits and customer use
cases.
</p>
<Link to="https://about.sourcegraph.com/batch-changes" rel="noopener">
{/*
a11y-ignore
Rule: "color-contrast" (Elements must have sufficient color contrast)
GitHub issue: https://github.com/sourcegraph/sourcegraph/issues/33343
*/}
<Link to="https://about.sourcegraph.com/batch-changes" rel="noopener" className="a11y-ignore">
Batch Changes marketing page
</Link>
</CardBody>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ exports[`ExtensionsQueryInputToolbar renders 1`] = `
class="my-3 divider"
/>
<button
aria-controls="menu--1"
aria-controls=""
aria-haspopup="true"
class="btn btnOutlineSecondary btnSm"
data-reach-menu-button=""
id="menu-button--menu"
id="menu-button-2"
type="button"
>
Show all
Expand Down
2 changes: 2 additions & 0 deletions client/web/src/integration/batches.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
ExternalServiceKind,
SharedGraphQlOperations,
} from '@sourcegraph/shared/src/graphql-operations'
import { accessibilityAudit } from '@sourcegraph/shared/src/testing/accessibility'
import { createDriverForTest, Driver } from '@sourcegraph/shared/src/testing/driver'
import { afterEachSaveScreenshotIfFailed } from '@sourcegraph/shared/src/testing/screenshotReporter'

Expand Down Expand Up @@ -472,6 +473,7 @@ describe('Batches', () => {
await driver.page.click('[data-testid="test-getting-started-btn"]')
await driver.page.waitForSelector('[data-testid="test-getting-started"]')
await percySnapshotWithVariants(driver.page, 'Batch changes getting started page')
await accessibilityAudit(driver.page)
})
})

Expand Down
14 changes: 7 additions & 7 deletions client/web/src/integration/repository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -396,7 +396,7 @@ describe('Repository', () => {

await driver.page.goto(driver.sourcegraphBaseUrl + '/' + repositoryName)

await driver.page.waitForSelector('header.test-tree-page-title')
await driver.page.waitForSelector('div.test-tree-page-title')

// Assert that the directory listing displays properly
await driver.page.waitForSelector('.test-tree-entries')
Expand Down Expand Up @@ -433,8 +433,8 @@ describe('Repository', () => {
await driver.page.waitForSelector('.test-repo-header-repo-link')
await driver.page.click('.test-repo-header-repo-link')

await driver.page.waitForSelector('header.test-tree-page-title')
await assertSelectorHasText('header.test-tree-page-title', shortRepositoryName)
await driver.page.waitForSelector('div.test-tree-page-title')
await assertSelectorHasText('div.test-tree-page-title', shortRepositoryName)
await driver.assertWindowLocation(repositorySourcegraphUrl)

await driver.findElementWithText(clickedCommit, {
Expand Down Expand Up @@ -570,8 +570,8 @@ describe('Repository', () => {

await driver.page.goto(driver.sourcegraphBaseUrl + repositorySourcegraphUrl)

await driver.page.waitForSelector('header.test-tree-page-title')
await assertSelectorHasText('header.test-tree-page-title', shortRepositoryName)
await driver.page.waitForSelector('div.test-tree-page-title')
await assertSelectorHasText('div.test-tree-page-title', shortRepositoryName)
await assertSelectorHasText('.test-tree-entry-file', 'readme.md')

await driver.page.waitForSelector('#monaco-query-input .view-lines')
Expand Down Expand Up @@ -618,8 +618,8 @@ describe('Repository', () => {

await driver.page.goto(driver.sourcegraphBaseUrl + repositorySourcegraphUrl)

await driver.page.waitForSelector('header.test-tree-page-title')
await assertSelectorHasText('header.test-tree-page-title', 'my org/repo with spaces')
await driver.page.waitForSelector('div.test-tree-page-title')
await assertSelectorHasText('div.test-tree-page-title', 'my org/repo with spaces')
await assertSelectorHasText('.test-tree-entry-file', 'readme.md')

// page.click() fails for some reason with Error: Node is either not visible or not an HTMLElement
Expand Down
1 change: 1 addition & 0 deletions client/web/src/nav/NavBar/NavDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ export const NavDropdown: React.FunctionComponent<NavDropdownProps> = ({ toggleI
navItemStyles.itemFocusable
)}
ref={menuButtonReference}
aria-label={isExpanded ? 'Hide search menu' : 'Show search menu'}
>
<span className={navItemStyles.itemFocusableContent}>
<Icon
Expand Down
7 changes: 6 additions & 1 deletion client/web/src/nav/StatusMessagesNavItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,12 @@ export class StatusMessagesNavItem extends React.PureComponent<Props, State> {

return (
<Popover isOpen={this.state.isOpen} onOpenChange={event => this.setState({ isOpen: event.isOpen })}>
<PopoverTrigger className="nav-link py-0 px-0 percy-hide chromatic-ignore" as={Button} variant="link">
<PopoverTrigger
className="nav-link py-0 px-0 percy-hide chromatic-ignore"
as={Button}
variant="link"
aria-label={this.state.isOpen ? 'Hide status messages' : 'Show status messages'}
>
{this.renderIcon()}
</PopoverTrigger>

Expand Down
10 changes: 6 additions & 4 deletions client/web/src/nav/__snapshots__/GlobalNavbar.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,12 @@ exports[`GlobalNavbar default 1`] = `
</span>
</a>
<button
aria-controls="menu--1"
aria-controls=""
aria-haspopup="true"
aria-label="Show search menu"
class="btn navDropdownIconButton itemFocusable"
data-reach-menu-button=""
id="menu-button--menu"
id="menu-button-2"
type="button"
>
<span
Expand Down Expand Up @@ -349,11 +350,12 @@ exports[`GlobalNavbar low-profile 1`] = `
</span>
</a>
<button
aria-controls="menu--3"
aria-controls=""
aria-haspopup="true"
aria-label="Show search menu"
class="btn navDropdownIconButton itemFocusable"
data-reach-menu-button=""
id="menu-button--menu--3"
id="menu-button-4"
type="button"
>
<span
Expand Down
4 changes: 2 additions & 2 deletions client/web/src/nav/__snapshots__/MenuNavItem.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
exports[`MenuNavItem add menu children 1`] = `
<DocumentFragment>
<button
aria-controls="menu--1"
aria-controls=""
aria-haspopup="true"
class="bg-transparent menuNavItem"
data-reach-menu-button=""
id="menu-button--menu"
id="menu-button-2"
type="button"
>
<svg
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
exports[`StatusMessagesNavItem no messages 1`] = `
<DocumentFragment>
<button
aria-label="Show status messages"
class="btn btnLink nav-link py-0 px-0 percy-hide chromatic-ignore"
type="button"
>
Expand Down Expand Up @@ -43,6 +44,7 @@ exports[`StatusMessagesNavItem no messages 1`] = `
exports[`StatusMessagesNavItem one CloningProgress message as non-site admin 1`] = `
<DocumentFragment>
<button
aria-label="Show status messages"
class="btn btnLink nav-link py-0 px-0 percy-hide chromatic-ignore"
type="button"
>
Expand Down Expand Up @@ -83,6 +85,7 @@ exports[`StatusMessagesNavItem one CloningProgress message as non-site admin 1`]
exports[`StatusMessagesNavItem one CloningProgress message as site admin 1`] = `
<DocumentFragment>
<button
aria-label="Show status messages"
class="btn btnLink nav-link py-0 px-0 percy-hide chromatic-ignore"
type="button"
>
Expand Down Expand Up @@ -123,6 +126,7 @@ exports[`StatusMessagesNavItem one CloningProgress message as site admin 1`] = `
exports[`StatusMessagesNavItem one ExternalServiceSyncError message as non-site admin 1`] = `
<DocumentFragment>
<button
aria-label="Show status messages"
class="btn btnLink nav-link py-0 px-0 percy-hide chromatic-ignore"
type="button"
>
Expand Down Expand Up @@ -163,6 +167,7 @@ exports[`StatusMessagesNavItem one ExternalServiceSyncError message as non-site
exports[`StatusMessagesNavItem one ExternalServiceSyncError message as site admin 1`] = `
<DocumentFragment>
<button
aria-label="Show status messages"
class="btn btnLink nav-link py-0 px-0 percy-hide chromatic-ignore"
type="button"
>
Expand Down Expand Up @@ -203,6 +208,7 @@ exports[`StatusMessagesNavItem one ExternalServiceSyncError message as site admi
exports[`StatusMessagesNavItem one SyncError message as non-site admin 1`] = `
<DocumentFragment>
<button
aria-label="Show status messages"
class="btn btnLink nav-link py-0 px-0 percy-hide chromatic-ignore"
type="button"
>
Expand Down Expand Up @@ -243,6 +249,7 @@ exports[`StatusMessagesNavItem one SyncError message as non-site admin 1`] = `
exports[`StatusMessagesNavItem one SyncError message as site admin 1`] = `
<DocumentFragment>
<button
aria-label="Show status messages"
class="btn btnLink nav-link py-0 px-0 percy-hide chromatic-ignore"
type="button"
>
Expand Down
4 changes: 2 additions & 2 deletions client/web/src/nav/__snapshots__/UserNavItem.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
exports[`UserNavItem simple 1`] = `
<DocumentFragment>
<button
aria-controls="menu--1"
aria-controls=""
aria-haspopup="true"
class="d-flex align-items-center text-decoration-none test-user-nav-item-toggle menuButton"
data-reach-menu-button=""
id="menu-button--menu"
id="menu-button-2"
type="button"
>
<div
Expand Down
11 changes: 7 additions & 4 deletions client/wildcard/src/components/Menu/Menu.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React, { ComponentType, forwardRef } from 'react'
import React, { ComponentType, forwardRef, useMemo } from 'react'

import { Menu as ReachMenu, MenuProps as ReachMenuProps } from '@reach/menu-button'
import { isFunction, noop } from 'lodash'
import { isFunction, noop, uniqueId } from 'lodash'

import { ForwardReferenceComponent } from '../..'
import { Popover } from '../Popover'
Expand All @@ -20,10 +20,13 @@ export type MenuProps = ReachMenuProps & {
* @see — Docs https://reach.tech/menu-button#menu
*/
export const Menu = forwardRef((props, reference) => {
const { children, as: Component, ...rest } = props
const { children, as: Component, id, ...rest } = props
// To fix Rule: "aria-valid-attr-value"
// Invalid ARIA attribute value: aria-controls="menu--1"
const uniqueAriaControlId = useMemo(() => id ?? uniqueId('menu-'), [id])

return (
<ReachMenu as={Component} ref={reference} {...rest}>
<ReachMenu as={Component} ref={reference} id={uniqueAriaControlId} {...rest}>
{({ isExpanded }) => (
<Popover isOpen={isExpanded} onOpenChange={noop}>
{isFunction(children) ? children({ isExpanded, isOpen: isExpanded }) : children}
Expand Down
21 changes: 15 additions & 6 deletions client/wildcard/src/components/Menu/MenuButton.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react'
import React, { useMemo } from 'react'

import { MenuButton as ReachMenuButton } from '@reach/menu-button'
import { uniqueId } from 'lodash'

import { ForwardReferenceComponent } from '../../types'
import { Button, ButtonProps } from '../Button'
Expand All @@ -14,11 +15,19 @@ export type MenuButtonProps = Omit<ButtonProps, 'as'>
*
* @see — Docs https://reach.tech/menu-button#menubutton
*/
export const MenuButton = React.forwardRef(({ children, ...props }, reference) => (
<ReachMenuButton ref={reference} as={PopoverTriggerButton} {...props}>
{children}
</ReachMenuButton>
)) as ForwardReferenceComponent<'button', MenuButtonProps>
export const MenuButton = React.forwardRef(({ children, id, ...props }, reference) => {
// To fix rule: "duplicate-id-active"
// Document has active elements with the same id attribute: menu-button--menu
const uniqueMenuId = useMemo(() => id ?? uniqueId('menu-button-'), [id])

// We unset aria-controls as it causes accessibility issues if the Menu is not yet rendered in the DOM.
// aria-controls has low support across screen readers so this shouldn't be an issue: https://github.com/w3c/aria/issues/995
return (
<ReachMenuButton ref={reference} as={PopoverTriggerButton} id={uniqueMenuId} {...props} aria-controls="">
{children}
</ReachMenuButton>
)
}) as ForwardReferenceComponent<'button', MenuButtonProps>

const PopoverTriggerButton = React.forwardRef((props, reference) => (
<PopoverTrigger ref={reference} as={Button} {...props} />
Expand Down
Loading