Merge branch 'main' into banner-fix

Author: langermank

.changeset/dark-birds-dont.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Fixes `Details` flickering, prevents re-renders

examples/codesandbox/package.json

@@ -20,6 +20,6 @@
     "@primer/react": "38.0.0-rc.8",
     "styled-components": "5.x",
     "typescript": "^5.9.2",
-    "vite": "^7.1.5"
+    "vite": "^7.1.11"
   }
 }

package-lock.json

@@ -23,16 +23,20 @@ export const Default = () => (
   </ButtonGroup>
 )
 
-export const Playground: StoryFn<ButtonProps> = args => (
-  <ButtonGroup>
-    <Button {...args}>Button 1</Button>
-    <Button {...args}>Button 2</Button>
-    <Button {...args}>Button 3</Button>
-  </ButtonGroup>
-)
+export const Playground: StoryFn<ButtonProps & {buttonCount: number}> = args => {
+  const {buttonCount = 3, ...buttonProps} = args
+  const buttons = Array.from({length: buttonCount}, (_, i) => (
+    <Button key={i} {...buttonProps}>
+      {`Button ${i + 1}`}
+    </Button>
+  ))
+
+  return <ButtonGroup>{buttons}</ButtonGroup>
+}
 Playground.args = {
   size: 'medium',
   disabled: false,
+  buttonCount: 3,
 }
 Playground.argTypes = {
   size: {
@@ -46,4 +50,13 @@ Playground.argTypes = {
       type: 'boolean',
     },
   },
+  buttonCount: {
+    control: {
+      type: 'number',
+      min: 2,
+      max: 6,
+      step: 1,
+    },
+    description: 'Number of buttons in the group (2-6)',
+  },
 }

packages/react/src/ButtonGroup/ButtonGroup.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<typeof Details>
+
+export const WithCustomSummary: StoryFn<typeof Details> = () => {
+  const {getDetailsProps} = useDetails({closeOnOutsideClick: true})
+  return (
+    <Details {...getDetailsProps()}>
+      <summary>Custom see Details</summary>
+      This is some content
+    </Details>
+  )
+}

packages/react/src/Details/Details.features.stories.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<HTMLDetailsElement, DetailsProps>(
   ({className, children, ...rest}, forwardRef): ReactElement => {
     const detailsRef = React.useRef<HTMLDetailsElement>(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 <Details> component must have a <summary> child component. You can either use <Details.Summary> or a native <summary> element.',
+      )
     }, [])
 
     return (
       <details className={clsx(className, classes.Details)} {...rest} ref={ref}>
-        {/* Include default summary if summary is not provided */}
-        {!hasSummary && <Details.Summary data-default-summary>{'See Details'}</Details.Summary>}
         {children}
       </details>
     )

packages/react/src/Details/Details.tsx

@@ -82,39 +82,6 @@ describe('Details', () => {
     expect(getByTestId('summary')).toHaveTextContent('Open')
   })
 
-  it('Adds default summary if no summary supplied', async () => {
-    const {getByText} = render(<Details data-testid="details">content</Details>)
-
-    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(
-      <Details data-testid="details">
-        <Details.Summary data-testid="summary">summary</Details.Summary>
-        content
-      </Details>,
-    )
-
-    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(
-      <Details data-testid="details">
-        <summary data-testid="summary">custom summary</summary>
-        content
-      </Details>,
-    )
-
-    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(<Details.Summary className="custom-class">test summary</Details.Summary>)