diff --git a/.changeset/rotten-mugs-tan.md b/.changeset/rotten-mugs-tan.md new file mode 100644 index 00000000000..4d252daf038 --- /dev/null +++ b/.changeset/rotten-mugs-tan.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +feat: make Spinner's delay customizable diff --git a/package-lock.json b/package-lock.json index 932603416c8..083c22b70b4 100644 --- a/package-lock.json +++ b/package-lock.json @@ -81,7 +81,7 @@ "react-dom": "^18.3.1" }, "devDependencies": { - "@primer/react": "38.7.0", + "@primer/react": "38.7.1", "@primer/styled-react": "1.0.2", "@types/react": "^18.3.11", "@types/react-dom": "^18.3.0", @@ -95,7 +95,7 @@ "name": "example-nextjs", "version": "0.0.0", "dependencies": { - "@primer/react": "38.7.0", + "@primer/react": "38.7.1", "@primer/styled-react": "1.0.2", "next": "^16.0.10", "react": "^19.2.0", @@ -138,7 +138,7 @@ "version": "0.0.0", "dependencies": { "@primer/octicons-react": "^19.21.0", - "@primer/react": "38.7.0", + "@primer/react": "38.7.1", "@primer/styled-react": "1.0.2", "clsx": "^2.1.1", "next": "^16.0.10", @@ -26428,7 +26428,7 @@ }, "packages/react": { "name": "@primer/react", - "version": "38.7.0", + "version": "38.7.1", "license": "MIT", "dependencies": { "@github/mini-throttle": "^2.1.1", diff --git a/packages/react/src/Spinner/Spinner.docs.json b/packages/react/src/Spinner/Spinner.docs.json index 4c71b59f068..a98a2e49c0d 100644 --- a/packages/react/src/Spinner/Spinner.docs.json +++ b/packages/react/src/Spinner/Spinner.docs.json @@ -33,9 +33,9 @@ }, { "defaultValue": "false", - "description": "Whether to delay the spinner before rendering by the defined 1000ms.", + "description": "Controls whether and how long to delay rendering the spinner. Set to `true` to delay by 1000ms, `'short'` to delay by 300ms, `'long'` to delay by 1000ms, or provide a custom number of milliseconds.", "name": "delay", - "type": "boolean" + "type": "boolean | 'short' | 'long' | number" } ], "status": "alpha", diff --git a/packages/react/src/Spinner/Spinner.features.stories.tsx b/packages/react/src/Spinner/Spinner.features.stories.tsx index 3d884d3193f..e27144c084b 100644 --- a/packages/react/src/Spinner/Spinner.features.stories.tsx +++ b/packages/react/src/Spinner/Spinner.features.stories.tsx @@ -20,4 +20,4 @@ export const SuppressScreenReaderText = () => ( ) -export const WithDelay = () => +export const WithDelay = () => diff --git a/packages/react/src/Spinner/Spinner.test.tsx b/packages/react/src/Spinner/Spinner.test.tsx index 735d7d61a81..caf2eefd776 100644 --- a/packages/react/src/Spinner/Spinner.test.tsx +++ b/packages/react/src/Spinner/Spinner.test.tsx @@ -62,6 +62,11 @@ describe('Spinner', () => { expect(container.querySelector('svg')).toBeInTheDocument() }) + it('should render immediately when no delay is provided', () => { + const {container} = render() + expect(container.querySelector('svg')).toBeInTheDocument() + }) + it('should not render immediately when delay is true', () => { const {container} = render() expect(container.querySelector('svg')).not.toBeInTheDocument() @@ -73,9 +78,96 @@ describe('Spinner', () => { // Not visible initially expect(container.querySelector('svg')).not.toBeInTheDocument() - // Advance timers by 1000ms + // Advance timers by less than 1000ms + act(() => { + vi.advanceTimersByTime(800) + }) + + // Still not visible + expect(container.querySelector('svg')).not.toBeInTheDocument() + + // Advance timers to complete the delay (1000ms) act(() => { - vi.advanceTimersByTime(1000) + vi.advanceTimersByTime(200) + }) + + // Now it should be visible + expect(container.querySelector('svg')).toBeInTheDocument() + }) + + it('should not render immediately when delay is "short"', () => { + const {container} = render() + expect(container.querySelector('svg')).not.toBeInTheDocument() + }) + + it('should render after 300ms when delay is "short"', () => { + const {container} = render() + + // Not visible initially + expect(container.querySelector('svg')).not.toBeInTheDocument() + + // Advance timers by less than 300ms + act(() => { + vi.advanceTimersByTime(250) + }) + + // Still not visible + expect(container.querySelector('svg')).not.toBeInTheDocument() + + // Advance timers to complete the short delay (300ms) + act(() => { + vi.advanceTimersByTime(50) + }) + + // Now it should be visible + expect(container.querySelector('svg')).toBeInTheDocument() + }) + + it('should not render immediately when delay is "long"', () => { + const {container} = render() + expect(container.querySelector('svg')).not.toBeInTheDocument() + }) + + it('should render after 1000ms when delay is "long"', () => { + const {container} = render() + + // Not visible initially + expect(container.querySelector('svg')).not.toBeInTheDocument() + + // Advance timers by less than 1000ms + act(() => { + vi.advanceTimersByTime(800) + }) + + // Still not visible + expect(container.querySelector('svg')).not.toBeInTheDocument() + + // Advance timers to complete the long delay (1000ms) + act(() => { + vi.advanceTimersByTime(200) + }) + + // Now it should be visible + expect(container.querySelector('svg')).toBeInTheDocument() + }) + + it('should render after custom ms when delay is a number', () => { + const {container} = render() + + // Not visible initially + expect(container.querySelector('svg')).not.toBeInTheDocument() + + // Advance timers by less than the custom delay (500ms) + act(() => { + vi.advanceTimersByTime(400) + }) + + // Still not visible + expect(container.querySelector('svg')).not.toBeInTheDocument() + + // Advance timers to complete the custom delay + act(() => { + vi.advanceTimersByTime(100) }) // Now it should be visible @@ -94,5 +186,44 @@ describe('Spinner', () => { // No errors should occur expect(true).toBe(true) }) + + it('should cleanup timeout on unmount when delay is "short"', () => { + const {unmount} = render() + + // Unmount before the delay completes + unmount() + + // Advance timers to see if there are any side effects + vi.advanceTimersByTime(300) + + // No errors should occur + expect(true).toBe(true) + }) + + it('should cleanup timeout on unmount when delay is "long"', () => { + const {unmount} = render() + + // Unmount before the delay completes + unmount() + + // Advance timers to see if there are any side effects + vi.advanceTimersByTime(1000) + + // No errors should occur + expect(true).toBe(true) + }) + + it('should cleanup timeout on unmount when delay is a number', () => { + const {unmount} = render() + + // Unmount before the delay completes + unmount() + + // Advance timers to see if there are any side effects + vi.advanceTimersByTime(500) + + // No errors should occur + expect(true).toBe(true) + }) }) }) diff --git a/packages/react/src/Spinner/Spinner.tsx b/packages/react/src/Spinner/Spinner.tsx index 73945964036..a212b580aa1 100644 --- a/packages/react/src/Spinner/Spinner.tsx +++ b/packages/react/src/Spinner/Spinner.tsx @@ -23,8 +23,8 @@ export type SpinnerProps = { 'aria-label'?: string className?: string style?: React.CSSProperties - /** Whether to delay the spinner before rendering by the defined 1000ms. */ - delay?: boolean + /** Controls whether and how long to delay rendering the spinner. Set to `true` to delay by 1000ms, `'short'` to delay by 300ms, `'long'` to delay by 1000ms, or provide a custom number of milliseconds. */ + delay?: boolean | 'short' | 'long' | number } & HTMLDataAttributes function Spinner({ @@ -46,9 +46,10 @@ function Spinner({ useEffect(() => { if (delay) { + const delayDuration = typeof delay === 'number' ? delay : delay === 'short' ? 300 : 1000 const timeoutId = setTimeout(() => { setIsVisible(true) - }, 1000) + }, delayDuration) return () => clearTimeout(timeoutId) }