From 3069da58f6937ed4b0a9c8dd1b864820fe614bb5 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Wed, 16 Oct 2024 22:59:59 +0000 Subject: [PATCH 01/14] Test elements accept className --- packages/react/src/utils/testing.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/react/src/utils/testing.tsx b/packages/react/src/utils/testing.tsx index 5f5f1caa257..836b05d21ad 100644 --- a/packages/react/src/utils/testing.tsx +++ b/packages/react/src/utils/testing.tsx @@ -224,6 +224,11 @@ export function behavesAsComponent({Component, toRender, options}: BehavesAsComp it('sets a valid displayName', () => { expect(Component.displayName).toMatch(COMPONENT_DISPLAY_NAME_REGEX) }) + + it('should support `className` on the outermost element', () => { + const elem = React.cloneElement(getElement(), {className: 'test-class-name'}) + expect(rendersClass(elem, 'test-class-name')).toBe(true) + }) } // eslint-disable-next-line @typescript-eslint/no-explicit-any From 60392ca5450ac548925b8554d7ddc3278e7efb28 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Wed, 16 Oct 2024 21:08:10 -0700 Subject: [PATCH 02/14] Create a test check for expectRendersWithClassname --- .../__tests__/TextInputWithTokens.test.tsx | 8 +++++- packages/react/src/utils/testing.tsx | 25 +++++++++++++++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/packages/react/src/__tests__/TextInputWithTokens.test.tsx b/packages/react/src/__tests__/TextInputWithTokens.test.tsx index 10a21222184..f450bc9ded0 100644 --- a/packages/react/src/__tests__/TextInputWithTokens.test.tsx +++ b/packages/react/src/__tests__/TextInputWithTokens.test.tsx @@ -1,5 +1,5 @@ import React from 'react' -import {render} from '../utils/testing' +import {expectRendersWithClassname, render} from '../utils/testing' import {render as HTMLRender, fireEvent, act} from '@testing-library/react' import axe from 'axe-core' import type {TokenSizeKeys} from '../Token/TokenBase' @@ -36,6 +36,12 @@ const LabelledTextInputWithTokens: React.FC { + it('should support `className` on the outermost element', () => { + const onRemoveMock = jest.fn() + const elem = + expectRendersWithClassname(elem, 'test-class-name') + }) + it('renders without tokens', () => { const onRemoveMock = jest.fn() expect(render()).toMatchSnapshot() diff --git a/packages/react/src/utils/testing.tsx b/packages/react/src/utils/testing.tsx index 836b05d21ad..6622e2f58ab 100644 --- a/packages/react/src/utils/testing.tsx +++ b/packages/react/src/utils/testing.tsx @@ -8,6 +8,7 @@ import customRules from '@github/axe-github' import {ThemeProvider} from '..' import {default as defaultTheme} from '../theme' import type {LiveRegionElement} from '@primer/live-region-element' +import {FeatureFlags} from '../FeatureFlags' type ComputedStyles = Record> @@ -224,11 +225,6 @@ export function behavesAsComponent({Component, toRender, options}: BehavesAsComp it('sets a valid displayName', () => { expect(Component.displayName).toMatch(COMPONENT_DISPLAY_NAME_REGEX) }) - - it('should support `className` on the outermost element', () => { - const elem = React.cloneElement(getElement(), {className: 'test-class-name'}) - expect(rendersClass(elem, 'test-class-name')).toBe(true) - }) } // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -240,6 +236,25 @@ export function checkExports(path: string, exports: Record): void { }) } +export function expectRendersWithClassname(element: React.ReactElement, className: string) { + const Element = () => element + const FeatureFlagElement = () => { + return ( + + + + ) + } + expect(HTMLRender().container.firstChild).toHaveClass(className) + expect(HTMLRender().container.firstChild).toHaveClass(className) +} + axe.configure(customRules) export function checkStoriesForAxeViolations(name: string, storyDir?: string) { From acb93bb62dc4436166fbcda5bd939992c3c96990 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Wed, 16 Oct 2024 21:10:56 -0700 Subject: [PATCH 03/14] Create three-jokes-bow.md --- .changeset/three-jokes-bow.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/three-jokes-bow.md diff --git a/.changeset/three-jokes-bow.md b/.changeset/three-jokes-bow.md new file mode 100644 index 00000000000..20a3dbe9e18 --- /dev/null +++ b/.changeset/three-jokes-bow.md @@ -0,0 +1,5 @@ +--- +"@primer/react": patch +--- + +Make sure all components accept `className` as a prop on outermost component element. From 07439138ae9cc19c02511a24dd8f87b4ad3770d9 Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Thu, 17 Oct 2024 04:57:45 +0000 Subject: [PATCH 04/14] Check test on Avatar --- packages/react/src/__tests__/Avatar.test.tsx | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/react/src/__tests__/Avatar.test.tsx b/packages/react/src/__tests__/Avatar.test.tsx index 10b902fea2a..3fe069301db 100644 --- a/packages/react/src/__tests__/Avatar.test.tsx +++ b/packages/react/src/__tests__/Avatar.test.tsx @@ -1,7 +1,7 @@ import React from 'react' import {Avatar} from '..' import theme from '../theme' -import {px, render, behavesAsComponent, checkExports} from '../utils/testing' +import {px, render, behavesAsComponent, checkExports, expectRendersWithClassname} from '../utils/testing' import {render as HTMLRender} from '@testing-library/react' import axe from 'axe-core' @@ -12,6 +12,11 @@ describe('Avatar', () => { default: Avatar, }) + it('should support `className` on the outermost element', () => { + const element = + expectRendersWithClassname(element, 'test-class-name') + }) + it('should have no axe violations', async () => { const {container} = HTMLRender() const results = await axe.run(container) From d1cd42a6df19f52cac5a86df4b4f0b76c3ab11bb Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Thu, 17 Oct 2024 05:01:14 +0000 Subject: [PATCH 05/14] Add test for AvatarPair --- packages/react/src/__tests__/AvatarStack.test.tsx | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/packages/react/src/__tests__/AvatarStack.test.tsx b/packages/react/src/__tests__/AvatarStack.test.tsx index c04aa672470..688b94137a1 100644 --- a/packages/react/src/__tests__/AvatarStack.test.tsx +++ b/packages/react/src/__tests__/AvatarStack.test.tsx @@ -1,6 +1,6 @@ import React from 'react' import {AvatarStack} from '..' -import {render, behavesAsComponent, checkExports} from '../utils/testing' +import {render, behavesAsComponent, checkExports, expectRendersWithClassname} from '../utils/testing' import {render as HTMLRender} from '@testing-library/react' import axe from 'axe-core' @@ -33,6 +33,18 @@ describe('Avatar', () => { default: AvatarStack, }) + it('should support `className` on the outermost element', () => { + const element = ( + + + + + + + ) + expectRendersWithClassname(element, 'test-class-name') + }) + it('should have no axe violations', async () => { const {container} = HTMLRender(avatarComp) const results = await axe.run(container) From 21dfd369745adc9cc1015239cd00ac4b64148afb Mon Sep 17 00:00:00 2001 From: Jon Rohan Date: Thu, 17 Oct 2024 05:21:57 +0000 Subject: [PATCH 06/14] Add more tests --- packages/react/src/Banner/Banner.test.tsx | 5 +++-- .../react/src/Breadcrumbs/__tests__/Breadcrumbs.test.tsx | 7 ++++++- packages/react/src/Button/__tests__/Button.test.tsx | 7 ++++++- packages/react/src/Checkbox/Checkbox.test.tsx | 7 ++++++- packages/react/src/CounterLabel/CounterLabel.test.tsx | 7 ++++++- packages/react/src/Heading/__tests__/Heading.test.tsx | 7 ++++++- packages/react/src/Link/__tests__/Link.test.tsx | 7 ++++++- packages/react/src/__tests__/Box.test.tsx | 7 ++++++- packages/react/src/__tests__/Label.test.tsx | 6 +++++- packages/react/src/__tests__/Popover.test.tsx | 7 ++++++- 10 files changed, 56 insertions(+), 11 deletions(-) diff --git a/packages/react/src/Banner/Banner.test.tsx b/packages/react/src/Banner/Banner.test.tsx index 7771c22cf3c..16a7a6473b9 100644 --- a/packages/react/src/Banner/Banner.test.tsx +++ b/packages/react/src/Banner/Banner.test.tsx @@ -2,6 +2,7 @@ import {render, screen} from '@testing-library/react' import userEvent from '@testing-library/user-event' import React from 'react' import {Banner} from '../Banner' +import {expectRendersWithClassname} from '../utils/testing' describe('Banner', () => { let spy: jest.SpyInstance @@ -30,8 +31,8 @@ describe('Banner', () => { }) it('should support a custom `className` on the outermost element', () => { - const {container} = render() - expect(container.firstChild).toHaveClass('test') + const element = + expectRendersWithClassname(element, 'test-class-name') }) it('should label the landmark element with the corresponding variant label text', () => { diff --git a/packages/react/src/Breadcrumbs/__tests__/Breadcrumbs.test.tsx b/packages/react/src/Breadcrumbs/__tests__/Breadcrumbs.test.tsx index 2c92fd0badf..f8358e8092d 100644 --- a/packages/react/src/Breadcrumbs/__tests__/Breadcrumbs.test.tsx +++ b/packages/react/src/Breadcrumbs/__tests__/Breadcrumbs.test.tsx @@ -1,6 +1,6 @@ import React from 'react' import Breadcrumbs, {Breadcrumb} from '..' -import {render, behavesAsComponent, checkExports} from '../../utils/testing' +import {render, behavesAsComponent, checkExports, expectRendersWithClassname} from '../../utils/testing' import {render as HTMLRender} from '@testing-library/react' import axe from 'axe-core' @@ -12,6 +12,11 @@ describe('Breadcrumbs', () => { Breadcrumb, }) + it('should support `className` on the outermost element', () => { + const element = + expectRendersWithClassname(element, 'test-class-name') + }) + it('should have no axe violations', async () => { const {container} = HTMLRender() const results = await axe.run(container) diff --git a/packages/react/src/Button/__tests__/Button.test.tsx b/packages/react/src/Button/__tests__/Button.test.tsx index 4705433bde0..52d1714557a 100644 --- a/packages/react/src/Button/__tests__/Button.test.tsx +++ b/packages/react/src/Button/__tests__/Button.test.tsx @@ -4,7 +4,7 @@ import axe from 'axe-core' import React from 'react' import {IconButton, Button, LinkButton} from '../../Button' import type {ButtonProps} from '../../Button' -import {behavesAsComponent} from '../../utils/testing' +import {behavesAsComponent, expectRendersWithClassname} from '../../utils/testing' import {FeatureFlags} from '../../FeatureFlags' type StatefulLoadingButtonProps = { @@ -31,6 +31,11 @@ describe('Button', () => { options: {skipSx: true, skipAs: true}, }) + it('should support `className` on the outermost element', () => { + const element = ) const button = container.getByRole('button') diff --git a/packages/react/src/Checkbox/Checkbox.test.tsx b/packages/react/src/Checkbox/Checkbox.test.tsx index 085fae85432..783c37bbc7f 100644 --- a/packages/react/src/Checkbox/Checkbox.test.tsx +++ b/packages/react/src/Checkbox/Checkbox.test.tsx @@ -2,7 +2,7 @@ import {render} from '@testing-library/react' import userEvent from '@testing-library/user-event' import React from 'react' import Checkbox from '../Checkbox' -import {behavesAsComponent, checkExports} from '../utils/testing' +import {behavesAsComponent, checkExports, expectRendersWithClassname} from '../utils/testing' describe('Checkbox', () => { beforeEach(() => { @@ -14,6 +14,11 @@ describe('Checkbox', () => { default: Checkbox, }) + it('should support `className` on the outermost element', () => { + const element = + expectRendersWithClassname(element, 'test-class-name') + }) + it('renders a valid checkbox input', () => { const {getByRole} = render() diff --git a/packages/react/src/CounterLabel/CounterLabel.test.tsx b/packages/react/src/CounterLabel/CounterLabel.test.tsx index 78ae5497bbe..cb3fd24d6d5 100644 --- a/packages/react/src/CounterLabel/CounterLabel.test.tsx +++ b/packages/react/src/CounterLabel/CounterLabel.test.tsx @@ -1,6 +1,6 @@ import React from 'react' import {CounterLabel} from '..' -import {behavesAsComponent, checkExports} from '../utils/testing' +import {behavesAsComponent, checkExports, expectRendersWithClassname} from '../utils/testing' import {render as HTMLRender} from '@testing-library/react' import axe from 'axe-core' @@ -11,6 +11,11 @@ describe('CounterLabel', () => { default: CounterLabel, }) + it('should support `className` on the outermost element', () => { + const element = + expectRendersWithClassname(element, 'test-class-name') + }) + it('renders a ', () => { const {container} = HTMLRender(1234) expect(container.firstChild?.nodeName).toEqual('SPAN') diff --git a/packages/react/src/Heading/__tests__/Heading.test.tsx b/packages/react/src/Heading/__tests__/Heading.test.tsx index 63a2484d050..ec800ad3af6 100644 --- a/packages/react/src/Heading/__tests__/Heading.test.tsx +++ b/packages/react/src/Heading/__tests__/Heading.test.tsx @@ -1,6 +1,6 @@ import React from 'react' import {Heading} from '../..' -import {render, behavesAsComponent, checkExports} from '../../utils/testing' +import {render, behavesAsComponent, checkExports, expectRendersWithClassname} from '../../utils/testing' import {render as HTMLRender, screen} from '@testing-library/react' import axe from 'axe-core' import ThemeProvider from '../../ThemeProvider' @@ -35,6 +35,11 @@ describe('Heading', () => { default: Heading, }) + it('should support `className` on the outermost element', () => { + const element = + expectRendersWithClassname(element, 'test-class-name') + }) + it('renders

by default', () => { expect(render().type).toEqual('h2') }) diff --git a/packages/react/src/Link/__tests__/Link.test.tsx b/packages/react/src/Link/__tests__/Link.test.tsx index fb5e59f2b32..b9061e7d5ff 100644 --- a/packages/react/src/Link/__tests__/Link.test.tsx +++ b/packages/react/src/Link/__tests__/Link.test.tsx @@ -1,6 +1,6 @@ import React from 'react' import Link from '..' -import {render, behavesAsComponent, checkExports} from '../../utils/testing' +import {render, behavesAsComponent, checkExports, expectRendersWithClassname} from '../../utils/testing' import {render as HTMLRender} from '@testing-library/react' import axe from 'axe-core' @@ -11,6 +11,11 @@ describe('Link', () => { default: Link, }) + it('should support `className` on the outermost element', () => { + const element = + expectRendersWithClassname(element, 'test-class-name') + }) + it('should have no axe violations', async () => { const {container} = HTMLRender(GitHub) const results = await axe.run(container) diff --git a/packages/react/src/__tests__/Box.test.tsx b/packages/react/src/__tests__/Box.test.tsx index edaf2fda5ab..03f57108e2d 100644 --- a/packages/react/src/__tests__/Box.test.tsx +++ b/packages/react/src/__tests__/Box.test.tsx @@ -3,7 +3,7 @@ import axe from 'axe-core' import React from 'react' import {Box} from '..' import theme from '../theme' -import {behavesAsComponent, checkExports, render} from '../utils/testing' +import {behavesAsComponent, checkExports, expectRendersWithClassname, render} from '../utils/testing' describe('Box', () => { behavesAsComponent({Component: Box}) @@ -12,6 +12,11 @@ describe('Box', () => { default: Box, }) + it('should support `className` on the outermost element', () => { + const element = + expectRendersWithClassname(element, 'test-class-name') + }) + it('should have no axe violations', async () => { const {container} = HTMLRender() const results = await axe.run(container) diff --git a/packages/react/src/__tests__/Label.test.tsx b/packages/react/src/__tests__/Label.test.tsx index ec4550880e7..ae247badbbb 100644 --- a/packages/react/src/__tests__/Label.test.tsx +++ b/packages/react/src/__tests__/Label.test.tsx @@ -3,9 +3,13 @@ import {render} from '@testing-library/react' import axe from 'axe-core' import type {LabelColorOptions} from '../Label' import Label, {variants} from '../Label' -import {renderStyles} from '../utils/testing' +import {expectRendersWithClassname, renderStyles} from '../utils/testing' describe('Label', () => { + it('should support `className` on the outermost element', () => { + const element =