fix(collapsible): code review updates, tests and types

adevinta · Jun 7, 2024 · 6c351bd · 6c351bd
1 parent caec9de
commit 6c351bd
Show file tree

Hide file tree

Showing 5 changed files with 9 additions and 16 deletions.
diff --git a/packages/components/collapsible/src/Collapsible.test.tsx b/packages/components/collapsible/src/Collapsible.test.tsx
@@ -11,9 +11,7 @@ describe('Collapsible', () => {
 
     render(
       <Collapsible>
-        <Collapsible.Trigger asChild>
-          <button>Expand</button>
-        </Collapsible.Trigger>
+        <Collapsible.Trigger>Expand</Collapsible.Trigger>
 
         <Collapsible.Content>
           <p>Collapsible content</p>
@@ -48,7 +46,6 @@ describe('Collapsible', () => {
 
     const trigger = screen.getByRole('button', { name: 'Expand' })
 
-    expect(trigger).toBeInTheDocument()
     expect(screen.getByText('Collapsible content')).not.toBeVisible()
 
     await user.click(trigger)
@@ -88,14 +85,14 @@ describe('Collapsible', () => {
 
       const trigger = screen.getByRole('button', { name: 'Expand' })
 
-      expect(trigger).toBeInTheDocument()
       expect(screen.getByText('Collapsible content')).not.toBeVisible()
       expect(onChange).not.toHaveBeenCalled()
 
       await user.click(trigger)
 
       expect(screen.getByText('Collapsible content')).toBeVisible()
       expect(onChange).toHaveBeenCalledWith(true)
+      expect(onChange).toHaveBeenCalledTimes(1)
     })
 
     it('should remain open when open is forced to `true`', async () => {
@@ -115,7 +112,6 @@ describe('Collapsible', () => {
 
       const trigger = screen.getByRole('button', { name: 'Expand' })
 
-      expect(trigger).toBeInTheDocument()
       expect(screen.getByText('Collapsible content')).toBeVisible()
 
       await user.click(trigger)
@@ -140,7 +136,6 @@ describe('Collapsible', () => {
 
       const trigger = screen.getByRole('button', { name: 'Expand' })
 
-      expect(trigger).toBeInTheDocument()
       expect(screen.getByText('Collapsible content')).not.toBeVisible()
 
       await user.click(trigger)

diff --git a/packages/components/collapsible/src/Collapsible.tsx b/packages/components/collapsible/src/Collapsible.tsx
@@ -5,7 +5,6 @@ import {
   type ComponentPropsWithoutRef,
   createContext,
   forwardRef,
-  type ReactNode,
   useContext,
   useId,
   useMemo,
@@ -16,8 +15,6 @@ export interface CollapsibleProps extends ComponentPropsWithoutRef<'div'> {
    * Change the default rendered element for the one passed as a child, merging their props and behavior.
    */
   asChild?: boolean
-  children: ReactNode
-  className?: string
   /**
    * The open state of the collapsible when it is initially rendered. Use when you do not need to control its open state.
    */
@@ -26,7 +23,6 @@ export interface CollapsibleProps extends ComponentPropsWithoutRef<'div'> {
    * When `true`, prevents the user from interacting with the collapsible.
    */
   disabled?: boolean
-  id?: string
   /**
    * Event handler called when the open state of the collapsible changes.
    */

diff --git a/packages/components/collapsible/src/CollapsibleContent.tsx b/packages/components/collapsible/src/CollapsibleContent.tsx
@@ -1,14 +1,12 @@
 import { Slot } from '@spark-ui/slot'
 import { mergeProps } from '@zag-js/react'
 import { cx } from 'class-variance-authority'
-import { type ComponentPropsWithoutRef, forwardRef, type ReactNode } from 'react'
+import { type ComponentPropsWithoutRef, forwardRef } from 'react'
 
 import { useCollapsibleContext } from './Collapsible'
 
-interface CollapsibleContentProps extends ComponentPropsWithoutRef<'div'> {
+export interface CollapsibleContentProps extends ComponentPropsWithoutRef<'div'> {
   asChild?: boolean
-  children: ReactNode
-  className?: string
 }
 
 export const Content = forwardRef<HTMLDivElement, CollapsibleContentProps>(

diff --git a/packages/components/collapsible/src/CollapsibleTrigger.tsx b/packages/components/collapsible/src/CollapsibleTrigger.tsx
@@ -4,7 +4,7 @@ import { type ComponentPropsWithoutRef, forwardRef, type ReactNode } from 'react
 
 import { useCollapsibleContext } from './Collapsible'
 
-interface CollapsibleTriggerProps extends ComponentPropsWithoutRef<'button'> {
+export interface CollapsibleTriggerProps extends ComponentPropsWithoutRef<'button'> {
   asChild?: boolean
   children: ReactNode
   className?: string

diff --git a/packages/components/collapsible/src/index.ts b/packages/components/collapsible/src/index.ts
@@ -15,3 +15,7 @@ export const Collapsible: FC<CollapsibleProps> & {
 Collapsible.displayName = 'Collapsible'
 Trigger.displayName = 'Collapsible.Trigger'
 Content.displayName = 'Collapsible.Content'
+
+export { type CollapsibleProps } from './Collapsible'
+export { type CollapsibleContentProps } from './CollapsibleContent'
+export { type CollapsibleTriggerProps } from './CollapsibleTrigger'