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

Commit

Permalink
test: enable and fix accessibilityAudit test issues in batches.test.ts
Browse files Browse the repository at this point in the history
  • Loading branch information
gitstart committed Apr 3, 2022
1 parent 7fcb906 commit 6a74626
Show file tree
Hide file tree
Showing 16 changed files with 72 additions and 28 deletions.
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,10 +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.
- `role="main"` to fix rule: "landmark-one-main" (Document should have one main landmark)
*/}
<div data-layout={true} className={classNames(styles.appRouterContainer, className)} {...rest}>
<div role="main" data-layout={true} className={classNames(styles.appRouterContainer, className)} {...rest}>
{children}
</div>
</ElementScroller>
Expand Down
8 changes: 7 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,13 @@ 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)
There are a lot of `Link` usages in `--color-bg-2=--gray-03` container, to fix this issue
we can use `--gray-02` or find another color for `--link-color`
*/}
<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
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 tasks status message' : 'Show tasks status message'}
>
{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 tasks status message"
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 tasks status message"
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 tasks status message"
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 tasks status message"
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 tasks status message"
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 tasks status message"
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 tasks status message"
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
23 changes: 17 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,21 @@ 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])

// aria-controls="" to fix "aria-valid-attr-value" since current version of
// Popover removes contents from DOM, it causes `aria-controls="menu-*"` invalid
// since `menu-*` doesn't exit on DOM

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
7 changes: 6 additions & 1 deletion client/wildcard/src/components/PageHeader/PageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,13 @@ export const PageHeader: React.FunctionComponent<Props> = ({
return null
}

/// a11y-ignore
/// Rule: "landmark-banner-is-top-level" (Banner landmark should not be contained in another landmark)
// We should think about stop using `header` here since we couldn't make it be be top-level
// with current usages

return (
<header className={classNames(styles.container, className)}>
<header className={classNames(styles.container, className, 'a11y-ignore')}>
<div>
{annotation && <small className={styles.annotation}>{annotation}</small>}
<HeadingX className={styles.heading}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ exports[`PageHeader renders correctly 1`] = `
<body>
<div>
<header
class="container"
class="container a11y-ignore"
>
<div>
<h1
Expand Down

0 comments on commit 6a74626

Please sign in to comment.