diff --git a/.changeset/dark-birds-dont.md b/.changeset/dark-birds-dont.md new file mode 100644 index 00000000000..a93b8f136d0 --- /dev/null +++ b/.changeset/dark-birds-dont.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Fixes `Details` flickering, prevents re-renders diff --git a/packages/react/src/Details/Details.features.stories.tsx b/packages/react/src/Details/Details.features.stories.tsx new file mode 100644 index 00000000000..d3cb107cddc --- /dev/null +++ b/packages/react/src/Details/Details.features.stories.tsx @@ -0,0 +1,18 @@ +import type {StoryFn, Meta} from '@storybook/react-vite' +import Details from './Details' +import useDetails from '../hooks/useDetails' + +export default { + title: 'Components/Details/Features', + component: Details, +} as Meta + +export const WithCustomSummary: StoryFn = () => { + const {getDetailsProps} = useDetails({closeOnOutsideClick: true}) + return ( +
+ Custom see Details + This is some content +
+ ) +} diff --git a/packages/react/src/Details/Details.tsx b/packages/react/src/Details/Details.tsx index 78353ae94b8..f493bf495e1 100644 --- a/packages/react/src/Details/Details.tsx +++ b/packages/react/src/Details/Details.tsx @@ -1,4 +1,5 @@ -import React, {useEffect, useState, type ComponentPropsWithoutRef, type ReactElement} from 'react' +import React, {useEffect, type ComponentPropsWithoutRef, type ReactElement} from 'react' +import {warning} from '../utils/warning' import {clsx} from 'clsx' import classes from './Details.module.css' import {useMergedRefs} from '../internal/hooks/useMergedRefs' @@ -7,40 +8,25 @@ const Root = React.forwardRef( ({className, children, ...rest}, forwardRef): ReactElement => { const detailsRef = React.useRef(null) const ref = useMergedRefs(forwardRef, detailsRef) - const [hasSummary, setHasSummary] = useState(false) useEffect(() => { - const {current: details} = detailsRef - if (!details) { + if (!__DEV__) { return } - const updateSummary = () => { - const summary = details.querySelector('summary:not([data-default-summary])') - setHasSummary(!!summary) - } - - // Update summary on mount - updateSummary() - - const observer = new MutationObserver(() => { - updateSummary() - }) - - observer.observe(details, { - childList: true, - subtree: true, - }) - - return () => { - observer.disconnect() + const {current: details} = detailsRef + if (!details) { + return } + const summary = details.querySelector('summary:not([data-default-summary])') + warning( + summary === null, + 'The
component must have a child component. You can either use or a native element.', + ) }, []) return (
- {/* Include default summary if summary is not provided */} - {!hasSummary && {'See Details'}} {children}
) diff --git a/packages/react/src/Details/__tests__/Details.test.tsx b/packages/react/src/Details/__tests__/Details.test.tsx index ef639556ee6..ead5d97a041 100644 --- a/packages/react/src/Details/__tests__/Details.test.tsx +++ b/packages/react/src/Details/__tests__/Details.test.tsx @@ -82,39 +82,6 @@ describe('Details', () => { expect(getByTestId('summary')).toHaveTextContent('Open') }) - it('Adds default summary if no summary supplied', async () => { - const {getByText} = render(
content
) - - expect(getByText('See Details')).toBeInTheDocument() - expect(getByText('See Details').tagName).toBe('SUMMARY') - }) - - it('Does not add default summary if summary supplied', async () => { - const {findByTestId, findByText} = render( -
- summary - content -
, - ) - - await expect(findByText('See Details')).rejects.toThrow() - expect(await findByTestId('summary')).toBeInTheDocument() - expect((await findByTestId('summary')).tagName).toBe('SUMMARY') - }) - - it('Does not add default summary if supplied as different element', async () => { - const {findByTestId, findByText} = render( -
- custom summary - content -
, - ) - - await expect(findByText('See Details')).rejects.toThrow() - expect(await findByTestId('summary')).toBeInTheDocument() - expect((await findByTestId('summary')).tagName).toBe('SUMMARY') - }) - describe('Details.Summary', () => { it('should support a custom `className` on the container element', () => { render(test summary)