Skip to content

Commit

Permalink
feat(Details): Add summary subcomponent (#5015)
Browse files Browse the repository at this point in the history
* fix(Details): Add summary prop and subcomponent

* Create old-boxes-clap.md

* fix: lint

* fix(Docs): details story use default export

* fix(Details): remove summary prop in favor of Details.Summary

* test(Details): fix breaking tests

* update changeset

* fix(Summary): add polymorphic type

* docs(Defatils): revert default story changes

* set output to true instead of files

* Update .changeset/old-boxes-clap.md

Co-authored-by: Josh Black <[email protected]>

* fix(Details): refactor summary detection logic

* fix(Details): remove console log

* test(Details): remove unused import

* fix(Details): use mutationObserver to track child changes

* test(Details): fix async tests

* fix(Details): remove summary warning

* fix(Summary): do not forward a ref

* fix(Summary): do not use sx prop

* fix(Summary): fix types

---------

Co-authored-by: Siddharth Kshetrapal <[email protected]>
Co-authored-by: Josh Black <[email protected]>
  • Loading branch information
3 people authored Oct 31, 2024
1 parent 8e58e4d commit 1473c26
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 18 deletions.
5 changes: 5 additions & 0 deletions .changeset/old-boxes-clap.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@primer/react": minor
---

feat(Details): Add summary subcomponent
23 changes: 22 additions & 1 deletion packages/react/src/Details/Details.docs.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,26 @@
"type": "SystemStyleObject"
}
],
"subcomponents": []
"subcomponents": [
{
"name": "Details.Summary",
"props": [
{
"name": "as",
"type": "React.ElementType<React.PropsWithChildren<any>>",
"defaultValue": "summary",
"required": false,
"description": "HTML element to render summary as."
},
{
"name": "children",
"type": "React.ReactNode"
},
{
"name": "sx",
"type": "SystemStyleObject"
}
]
}
]
}
2 changes: 1 addition & 1 deletion packages/react/src/Details/Details.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const Default: StoryFn<typeof Details> = () => {
const {getDetailsProps} = useDetails({closeOnOutsideClick: true})
return (
<Details {...getDetailsProps()}>
<Button as="summary">See Details</Button>
<Details.Summary as={Button}>See Details</Details.Summary>
This is some content
</Details>
)
Expand Down
67 changes: 63 additions & 4 deletions packages/react/src/Details/Details.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import React, {type ComponentPropsWithoutRef, type ReactElement} from 'react'
import React, {useEffect, useState, type ComponentPropsWithoutRef, type ReactElement} from 'react'
import styled from 'styled-components'
import type {SxProp} from '../sx'
import sx from '../sx'
import {toggleStyledComponent} from '../internal/utils/toggleStyledComponent'
import {useFeatureFlag} from '../FeatureFlags'
import {clsx} from 'clsx'
import classes from './Details.module.css'
import {useMergedRefs} from '../internal/hooks/useMergedRefs'

const CSS_MODULES_FEATURE_FLAG = 'primer_react_css_modules_team'

Expand All @@ -24,18 +25,76 @@ const StyledDetails = toggleStyledComponent(
`,
)

const Details = React.forwardRef<HTMLDetailsElement, DetailsProps>(
({className, children, ...rest}, ref): ReactElement => {
const Root = React.forwardRef<HTMLDetailsElement, DetailsProps>(
({className, children, ...rest}, forwardRef): ReactElement => {
const enabled = useFeatureFlag(CSS_MODULES_FEATURE_FLAG)
const detailsRef = React.useRef<HTMLDetailsElement>(null)
const ref = useMergedRefs(forwardRef, detailsRef)
const [hasSummary, setHasSummary] = useState(false)

useEffect(() => {
const {current: details} = detailsRef
if (!details) {
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()
}
}, [])

return (
<StyledDetails className={clsx(className, {[classes.Details]: enabled})} {...rest} ref={ref}>
{/* Include default summary if summary is not provided */}
{!hasSummary && <Details.Summary data-default-summary>{'See Details'}</Details.Summary>}
{children}
</StyledDetails>
)
},
)

Details.displayName = 'Details'
Root.displayName = 'Details'

export type SummaryProps<As extends React.ElementType> = {
/**
* HTML element to render summary as.
*/
as?: As
children?: React.ReactNode
} & React.ComponentPropsWithoutRef<React.ElementType extends As ? As : 'summary'>

function Summary<As extends React.ElementType>({as, children, ...props}: SummaryProps<As>) {
const Component = as ?? 'summary'
return (
<Component as={Component === 'summary' ? null : 'summary'} {...props}>
{children}
</Component>
)
}
Summary.displayName = 'Summary'

export {Summary}

const Details = Object.assign(Root, {
Summary,
})

export type DetailsProps = ComponentPropsWithoutRef<'details'> & SxProp
export default Details
80 changes: 68 additions & 12 deletions packages/react/src/Details/__tests__/Details.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {render} from '@testing-library/react'
import {render, screen} from '@testing-library/react'
import userEvent from '@testing-library/user-event'
import React from 'react'
import React, {act} from 'react'
import {Details, useDetails, Box, Button} from '../..'
import type {ButtonProps} from '../../Button'
import {behavesAsComponent, checkExports} from '../../utils/testing'
Expand All @@ -14,29 +14,36 @@ describe('Details', () => {
})

it('should have no axe violations', async () => {
const {container} = render(<Details />)
const results = await axe.run(container)
const {container} = render(
<Details>
<Details.Summary>Summary</Details.Summary>Content
</Details>,
)
let results
await act(async () => {
results = await axe.run(container)
})
expect(results).toHaveNoViolations()
})

it('Toggles when you click outside', () => {
it('Toggles when you click outside', async () => {
const Component = () => {
const {getDetailsProps} = useDetails({closeOnOutsideClick: true})
return (
<Details data-testid="details" {...getDetailsProps()}>
<summary>hi</summary>
<Details.Summary>hi</Details.Summary>
</Details>
)
}

const {getByTestId} = render(<Component />)
const {findByTestId} = render(<Component />)

document.body.click()

expect(getByTestId('details')).not.toHaveAttribute('open')
expect(await findByTestId('details')).not.toHaveAttribute('open')
})

it('Accurately passes down open state', () => {
it('Accurately passes down open state', async () => {
const Component = () => {
const {getDetailsProps, open} = useDetails({closeOnOutsideClick: true})
return (
Expand All @@ -46,12 +53,12 @@ describe('Details', () => {
)
}

const {getByTestId} = render(<Component />)
const {findByTestId} = render(<Component />)

document.body.click()

expect(getByTestId('summary')).toHaveTextContent('Closed')
expect(getByTestId('details')).not.toHaveAttribute('open')
expect(await findByTestId('summary')).toHaveTextContent('Closed')
expect(await findByTestId('details')).not.toHaveAttribute('open')
})

it('Can manipulate state with setOpen', async () => {
Expand Down Expand Up @@ -95,4 +102,53 @@ 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">
<Box as="summary" data-testid="summary">
custom summary
</Box>
content
</Details>,
)

await expect(findByText('See Details')).rejects.toThrow()
expect(await findByTestId('summary')).toBeInTheDocument()
expect((await findByTestId('summary')).tagName).toBe('SUMMARY')
})

describe('Details.Summary', () => {
behavesAsComponent({Component: Details.Summary, options: {skipSx: true}})

it('should support a custom `className` on the container element', () => {
render(<Details.Summary className="custom-class">test summary</Details.Summary>)
expect(screen.getByText('test summary')).toHaveClass('custom-class')
})

it('should pass extra props onto the container element', () => {
render(<Details.Summary data-testid="test">test summary</Details.Summary>)
expect(screen.getByText('test summary')).toHaveAttribute('data-testid', 'test')
})
})
})

0 comments on commit 1473c26

Please sign in to comment.