From 6c351bd92a32968c62d77312dacc7bea29825f8e Mon Sep 17 00:00:00 2001
From: Powerplex <julien.bonnin.dev@gmail.com>
Date: Fri, 7 Jun 2024 10:27:54 +0200
Subject: [PATCH] fix(collapsible): code review updates, tests and types

---
 packages/components/collapsible/src/Collapsible.test.tsx | 9 ++-------
 packages/components/collapsible/src/Collapsible.tsx      | 4 ----
 .../components/collapsible/src/CollapsibleContent.tsx    | 6 ++----
 .../components/collapsible/src/CollapsibleTrigger.tsx    | 2 +-
 packages/components/collapsible/src/index.ts             | 4 ++++
 5 files changed, 9 insertions(+), 16 deletions(-)

diff --git a/packages/components/collapsible/src/Collapsible.test.tsx b/packages/components/collapsible/src/Collapsible.test.tsx
index e7d1c674f..3621f1726 100644
--- 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,7 +85,6 @@ describe('Collapsible', () => {
 
       const trigger = screen.getByRole('button', { name: 'Expand' })
 
-      expect(trigger).toBeInTheDocument()
       expect(screen.getByText('Collapsible content')).not.toBeVisible()
       expect(onChange).not.toHaveBeenCalled()
 
@@ -96,6 +92,7 @@ describe('Collapsible', () => {
 
       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
index 099f0ad25..7fffac7e2 100644
--- 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
index 38c7af67d..720bd9d0c 100644
--- 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
index ce26fe9ec..e27d329f6 100644
--- 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
index 34487b215..767dac5bd 100644
--- 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'