From aeff5f7c71a9b44a70cb3044890fb424087158d3 Mon Sep 17 00:00:00 2001 From: Motasim Rahman Date: Sun, 10 Aug 2025 17:17:07 +0530 Subject: [PATCH 1/2] refactor: update Accordion component to use string values and enhance context API - Changed activeItems and itemValue types from (number | string) to string for consistency. - Introduced new properties in AccordionContext: type, collapsible, disabled, dir, forceMount, and hiddenUntilFound. - Updated AccordionItem and AccordionContent to accommodate new context properties. - Enhanced AccordionTrigger logic to respect collapsible behavior. - Adjusted stories and tests to reflect changes in value handling and ensure compatibility with Radix UI. --- .../Accordion/contexts/AccordionContext.tsx | 18 +++- .../contexts/AccordionItemContext.tsx | 6 +- .../Accordion/fragments/AccordionContent.tsx | 56 +++++++---- .../ui/Accordion/fragments/AccordionItem.tsx | 46 +++++---- .../ui/Accordion/fragments/AccordionRoot.tsx | 98 ++++++++++++++++--- .../Accordion/fragments/AccordionTrigger.tsx | 49 +++++++--- .../Accordion/stories/Accordion.stories.tsx | 38 +++---- .../AccordionRootVisualTests.stories.tsx | 12 +-- .../ui/Accordion/tests/Accordion.test.tsx | 16 +-- .../ui/Skeleton/stories/Skeleton.stories.tsx | 2 +- 10 files changed, 235 insertions(+), 106 deletions(-) diff --git a/src/components/ui/Accordion/contexts/AccordionContext.tsx b/src/components/ui/Accordion/contexts/AccordionContext.tsx index 03a3d9ce4..0d67d0447 100644 --- a/src/components/ui/Accordion/contexts/AccordionContext.tsx +++ b/src/components/ui/Accordion/contexts/AccordionContext.tsx @@ -2,12 +2,17 @@ import { createContext } from 'react'; interface AccordionContextType { rootClass?: string | null; - activeItems: (number | string)[]; - setActiveItems: (items: (number | string)[]) => void; + activeItems: string[]; + setActiveItems: (items: string[]) => void; accordionRef?: React.RefObject; transitionDuration?: number; transitionTimingFunction?: string; - openMultiple?: boolean; + type?: 'single' | 'multiple'; + collapsible?: boolean; + disabled?: boolean; + dir?: 'ltr' | 'rtl'; + forceMount?: boolean; + hiddenUntilFound?: boolean; } export const AccordionContext = createContext({ @@ -17,5 +22,10 @@ export const AccordionContext = createContext({ accordionRef: undefined, transitionDuration: 0, transitionTimingFunction: 'ease-out', - openMultiple: false + type: 'single', + collapsible: true, + disabled: false, + dir: 'ltr', + forceMount: false, + hiddenUntilFound: false }); diff --git a/src/components/ui/Accordion/contexts/AccordionItemContext.tsx b/src/components/ui/Accordion/contexts/AccordionItemContext.tsx index 682bb77b3..2808b6e67 100644 --- a/src/components/ui/Accordion/contexts/AccordionItemContext.tsx +++ b/src/components/ui/Accordion/contexts/AccordionItemContext.tsx @@ -1,13 +1,13 @@ import { createContext } from 'react'; interface AccordionItemContextType { - itemValue: number | string; - setItemValue: (value: number | string) => void; + itemValue: string; + setItemValue: (value: string) => void; disabled: boolean; } export const AccordionItemContext = createContext({ - itemValue: 0, + itemValue: '', setItemValue: () => {}, disabled: false }); diff --git a/src/components/ui/Accordion/fragments/AccordionContent.tsx b/src/components/ui/Accordion/fragments/AccordionContent.tsx index e353885e4..e787b0820 100644 --- a/src/components/ui/Accordion/fragments/AccordionContent.tsx +++ b/src/components/ui/Accordion/fragments/AccordionContent.tsx @@ -6,32 +6,50 @@ import { AccordionItemContext } from '../contexts/AccordionItemContext'; import CollapsiblePrimitive from '~/core/primitives/Collapsible'; -type AccordionContentProps = { - children: React.ReactNode; - index: number, - className? :string +export type AccordionContentProps = { + children: React.ReactNode; + index?: number; + className?: string; + forceMount?: boolean; + asChild?: boolean; }; -const AccordionContent: React.FC = ({ children, index, className = '' }: AccordionContentProps) => { - const { activeItems, rootClass } = useContext(AccordionContext); +const AccordionContent: React.FC = ({ + children, + index, + className = '', + forceMount = false, + asChild = false +}: AccordionContentProps) => { + const { + activeItems, + rootClass, + hiddenUntilFound, + forceMount: rootForceMount + } = useContext(AccordionContext); const { itemValue } = useContext(AccordionItemContext); - // Use itemValue to determine if this content should be visible const isOpen = activeItems.includes(itemValue); + const shouldRender = forceMount || rootForceMount || isOpen; + const shouldHide = hiddenUntilFound && !isOpen; + + if (!shouldRender) { + return null; + } return ( - isOpen - ? - {children} - - : null + + {children} + ); }; diff --git a/src/components/ui/Accordion/fragments/AccordionItem.tsx b/src/components/ui/Accordion/fragments/AccordionItem.tsx index 1e6bba7ec..68cd35811 100644 --- a/src/components/ui/Accordion/fragments/AccordionItem.tsx +++ b/src/components/ui/Accordion/fragments/AccordionItem.tsx @@ -10,51 +10,61 @@ import Primitive from '~/core/primitives/Primitive'; export type AccordionItemProps = { children: React.ReactNode; className?: string; - value?: number | string; + value: string; // Required for Radix UI compatibility disabled?: boolean; asChild?: boolean; } -const AccordionItem: React.FC = ({ children, value, className = '', disabled = false, asChild = false, ...props }) => { +const AccordionItem: React.FC = ({ + children, + value, + className = '', + disabled = false, + asChild = false, + ...props +}) => { const accordionItemRef = useRef(null); - const [itemValue, setItemValue] = useState(value ?? 0); - const { rootClass, activeItems, transitionDuration, transitionTimingFunction } = useContext(AccordionContext); + const { + rootClass, + activeItems, + transitionDuration, + transitionTimingFunction, + disabled: rootDisabled, + dir + } = useContext(AccordionContext); + + const [isOpen, setIsOpen] = useState(activeItems.includes(value)); + const isDisabled = rootDisabled || disabled; - const [isOpen, setIsOpen] = useState(activeItems.includes(itemValue)); useEffect(() => { - setIsOpen(activeItems.includes(itemValue)); - }, [activeItems, itemValue]); + setIsOpen(activeItems.includes(value)); + }, [activeItems, value]); const id = useId(); - // Update itemValue if value prop changes - useEffect(() => { - if (value !== undefined && value !== itemValue) { - setItemValue(value); - } - }, [value]); - return ( - + {}, disabled: isDisabled }}> {children} - ); }; diff --git a/src/components/ui/Accordion/fragments/AccordionRoot.tsx b/src/components/ui/Accordion/fragments/AccordionRoot.tsx index cb11bc0d8..7b1663155 100644 --- a/src/components/ui/Accordion/fragments/AccordionRoot.tsx +++ b/src/components/ui/Accordion/fragments/AccordionRoot.tsx @@ -18,43 +18,109 @@ export type AccordionRootProps = { asChild?: boolean; loop?: boolean; disableTabIndexing?: boolean; + type?: 'single' | 'multiple'; + collapsible?: boolean; + disabled?: boolean; + dir?: 'ltr' | 'rtl'; + forceMount?: boolean; + hiddenUntilFound?: boolean; + // Legacy props (deprecated) openMultiple?: boolean; - value?: (number | string)[]; - defaultValue?: (number | string)[]; - onValueChange?: (value: (number | string)[]) => void; + keepMounted?: boolean; + // Value props (updated for Radix UI compatibility) + value?: string | string[]; + defaultValue?: string | string[]; + onValueChange?: (value: string | string[]) => void; } -const AccordionRoot = ({ children, orientation = 'vertical', disableTabIndexing = true, asChild, transitionDuration = 0, transitionTimingFunction = 'linear', customRootClass, loop = true, openMultiple = false, value, defaultValue = [], onValueChange }: AccordionRootProps) => { +const AccordionRoot = ({ + children, + orientation = 'vertical', + disableTabIndexing = true, + asChild, + transitionDuration = 0, + transitionTimingFunction = 'linear', + customRootClass, + loop = true, + type = 'single', + collapsible = true, + disabled = false, + dir = 'ltr', + forceMount = false, + hiddenUntilFound = false, + // Legacy props (deprecated) + openMultiple, + keepMounted, + // Value props + value, + defaultValue = [], + onValueChange +}: AccordionRootProps) => { const accordionRef = useRef(null); const rootClass = customClassSwitcher(customRootClass, COMPONENT_NAME); + + // Handle legacy props for backward compatibility + const actualType = openMultiple !== undefined ? (openMultiple ? 'multiple' : 'single') : type; + const actualForceMount = keepMounted !== undefined ? keepMounted : forceMount; + + // Process values based on type const processedValue = value !== undefined - ? (openMultiple ? value : (value.length > 0 ? [value[0]] : [])) + ? (actualType === 'multiple' + ? (Array.isArray(value) ? value : [value]) + : (Array.isArray(value) ? (value.length > 0 ? [value[0]] : []) : [value])) : undefined; - const processedDefaultValue = openMultiple - ? defaultValue - : (defaultValue.length > 0 ? [defaultValue[0]] : []); + const processedDefaultValue = actualType === 'multiple' + ? (Array.isArray(defaultValue) ? defaultValue : [defaultValue]) + : (Array.isArray(defaultValue) ? (defaultValue.length > 0 ? [defaultValue[0]] : []) : [defaultValue]); - const [activeItems, setActiveItems] = useControllableState<(number | string)[]>( + const [activeItems, setActiveItems] = useControllableState( processedValue, - processedDefaultValue, - onValueChange + processedDefaultValue, + onValueChange ); + // Handle collapsible logic (only applies to single type) + const handleValueChange = (newValue: string[]) => { + if (actualType === 'single' && !collapsible && newValue.length === 0) { + // Prevent closing all items when collapsible is false + console.warn('Accordion: Cannot close all items when collapsible is false'); + return; + } + setActiveItems(newValue); + }; + return ( - - - + + + {children} diff --git a/src/components/ui/Accordion/fragments/AccordionTrigger.tsx b/src/components/ui/Accordion/fragments/AccordionTrigger.tsx index c5675817c..472c23adb 100644 --- a/src/components/ui/Accordion/fragments/AccordionTrigger.tsx +++ b/src/components/ui/Accordion/fragments/AccordionTrigger.tsx @@ -8,31 +8,49 @@ import CollapsiblePrimitive from '~/core/primitives/Collapsible'; import RovingFocusGroup from '~/core/utils/RovingFocusGroup'; import ButtonPrimitive from '~/core/primitives/Button'; -type AccordionTriggerProps = { - children: React.ReactNode; - className?: string, - index?: number, - handleClick?: (index: number) => void +export type AccordionTriggerProps = { + children: React.ReactNode; + className?: string; + index?: number; + handleClick?: (index: number) => void; }; -const AccordionTrigger: React.FC = ({ children, index, className = '' }) => { +const AccordionTrigger: React.FC = ({ + children, + index, + className = '' +}) => { const triggerRef = useRef(null); - const { setActiveItems, rootClass, activeItems, openMultiple } = useContext(AccordionContext); - const { itemValue, disabled } = useContext(AccordionItemContext); + const { + setActiveItems, + rootClass, + activeItems, + type, + collapsible, + disabled: rootDisabled + } = useContext(AccordionContext); + const { itemValue, disabled: itemDisabled } = useContext(AccordionItemContext); + + const isDisabled = rootDisabled || itemDisabled; const onClickHandler = (e: React.MouseEvent) => { - // Create safe array regardless of activeItems state + if (isDisabled) return; + const currentActiveItems = activeItems || []; - if (openMultiple) { - // check if the item is already part of the array + if (type === 'multiple') { if (currentActiveItems.includes(itemValue)) { - setActiveItems(currentActiveItems.filter((item) => item !== itemValue)); + const newItems = currentActiveItems.filter((item) => item !== itemValue); + setActiveItems(newItems); } else { setActiveItems([...currentActiveItems, itemValue]); } } else { if (currentActiveItems.includes(itemValue)) { + // Check collapsible constraint + if (!collapsible) { + return; + } setActiveItems([]); } else { setActiveItems([itemValue]); @@ -42,14 +60,15 @@ const AccordionTrigger: React.FC = ({ children, index, cl return ( - + {children} diff --git a/src/components/ui/Accordion/stories/Accordion.stories.tsx b/src/components/ui/Accordion/stories/Accordion.stories.tsx index e88ef6aa8..1e29803df 100644 --- a/src/components/ui/Accordion/stories/Accordion.stories.tsx +++ b/src/components/ui/Accordion/stories/Accordion.stories.tsx @@ -70,7 +70,7 @@ const AccordionExample = ({ ...args }) => { return ( {items.map((item, index) => ( - + {item.title} @@ -98,24 +98,26 @@ export const OpenMultiple: Story = { render: () => }; -export const WithDeafultValue: Story = { - render: () => +export const TypeMultiple: Story = { + render: () => }; -export const ControlledValue: Story = { - render: () => { - const [value, setValue] = React.useState([]); - const [multiple, setMultiple] = React.useState(false); +export const NonCollapsible: Story = { + render: () => +}; - return ( - <> - - - - - - - - ); - } +export const Disabled: Story = { + render: () => +}; + +export const RTL: Story = { + render: () => +}; + +export const ForceMount: Story = { + render: () => +}; + +export const HiddenUntilFound: Story = { + render: () => }; diff --git a/src/components/ui/Accordion/stories/AccordionRootVisualTests.stories.tsx b/src/components/ui/Accordion/stories/AccordionRootVisualTests.stories.tsx index c1da1cf9f..48b21af80 100644 --- a/src/components/ui/Accordion/stories/AccordionRootVisualTests.stories.tsx +++ b/src/components/ui/Accordion/stories/AccordionRootVisualTests.stories.tsx @@ -10,7 +10,7 @@ export default { const Template: Story = (args) => - + Hello @@ -31,7 +31,7 @@ const AsChildTemplate: Story = (args) => { - + Hello @@ -51,7 +51,7 @@ const HorizontalTemplate: Story = (args) => { return ( - + Item 1 @@ -59,7 +59,7 @@ const HorizontalTemplate: Story = (args) => { abc - + Item 2 @@ -78,7 +78,7 @@ const LoopOffTemplate: Story = (args) => { return ( - + Item 1 @@ -86,7 +86,7 @@ const LoopOffTemplate: Story = (args) => { abc - + Item 2 diff --git a/src/components/ui/Accordion/tests/Accordion.test.tsx b/src/components/ui/Accordion/tests/Accordion.test.tsx index 06cb980a1..794504227 100644 --- a/src/components/ui/Accordion/tests/Accordion.test.tsx +++ b/src/components/ui/Accordion/tests/Accordion.test.tsx @@ -18,7 +18,7 @@ const TestAccordion = (props: Partial) => { return ( {testItems.map((item, index) => ( - + {item.title} @@ -110,14 +110,18 @@ describe('Accordion Component', () => { test('controlled mode responds to external value changes', () => { const TestWithControls = () => { - const [value, setValue] = React.useState<(number | string)[]>([]); + const [value, setValue] = React.useState([]); + + const handleValueChange = (newValue: string | string[]) => { + setValue(Array.isArray(newValue) ? newValue : [newValue]); + }; return ( <> - - - + + + ); }; @@ -149,7 +153,7 @@ describe('Accordion Component', () => { }); test('works with defaultValue to show initial item', () => { - render(); + render(); // Item 3 content should be visible initially expect(screen.getByText('Content 3')).toBeInTheDocument(); diff --git a/src/components/ui/Skeleton/stories/Skeleton.stories.tsx b/src/components/ui/Skeleton/stories/Skeleton.stories.tsx index d04dd41fd..8292636b7 100644 --- a/src/components/ui/Skeleton/stories/Skeleton.stories.tsx +++ b/src/components/ui/Skeleton/stories/Skeleton.stories.tsx @@ -108,7 +108,7 @@ export default { {items.map((item, index) => ( - + {item.title} From 7b6ece0ae11b940ebed1d4d5a7bb12e7c056fe38 Mon Sep 17 00:00:00 2001 From: Motasim Rahman Date: Sun, 10 Aug 2025 18:25:50 +0530 Subject: [PATCH 2/2] refactor: enhance Accordion component functionality and improve accessibility - Added useRef and useEffect hooks in AccordionContent to manage the hidden attribute for better accessibility. - Simplified state management in AccordionItem by removing unnecessary useState and useEffect hooks. - Updated onValueChange type in AccordionRoot to accept undefined, improving type safety. - Adjusted AccordionItem to directly use activeItems for determining open state. --- .../Accordion/fragments/AccordionContent.tsx | 19 +++++++++++++++---- .../ui/Accordion/fragments/AccordionItem.tsx | 9 ++------- .../ui/Accordion/fragments/AccordionRoot.tsx | 6 ++++-- 3 files changed, 21 insertions(+), 13 deletions(-) diff --git a/src/components/ui/Accordion/fragments/AccordionContent.tsx b/src/components/ui/Accordion/fragments/AccordionContent.tsx index e787b0820..86764ac2a 100644 --- a/src/components/ui/Accordion/fragments/AccordionContent.tsx +++ b/src/components/ui/Accordion/fragments/AccordionContent.tsx @@ -1,6 +1,6 @@ 'use client'; import { clsx } from 'clsx'; -import React, { useContext } from 'react'; +import React, { useContext, useRef, useEffect } from 'react'; import { AccordionContext } from '../contexts/AccordionContext'; import { AccordionItemContext } from '../contexts/AccordionItemContext'; @@ -28,24 +28,35 @@ const AccordionContent: React.FC = ({ forceMount: rootForceMount } = useContext(AccordionContext); const { itemValue } = useContext(AccordionItemContext); + const contentRef = useRef(null); const isOpen = activeItems.includes(itemValue); const shouldRender = forceMount || rootForceMount || isOpen; const shouldHide = hiddenUntilFound && !isOpen; + useEffect(() => { + if (contentRef.current) { + if (shouldHide) { + contentRef.current.setAttribute('hidden', 'until-found'); + } else { + contentRef.current.removeAttribute('hidden'); + } + } + }, [shouldHide]); + if (!shouldRender) { return null; } return ( {children} diff --git a/src/components/ui/Accordion/fragments/AccordionItem.tsx b/src/components/ui/Accordion/fragments/AccordionItem.tsx index 68cd35811..b13c60997 100644 --- a/src/components/ui/Accordion/fragments/AccordionItem.tsx +++ b/src/components/ui/Accordion/fragments/AccordionItem.tsx @@ -1,5 +1,5 @@ 'use client'; -import React, { useState, useContext, useId, useEffect, useRef } from 'react'; +import React, { useContext, useId, useRef } from 'react'; import { clsx } from 'clsx'; import { AccordionContext } from '../contexts/AccordionContext'; import { AccordionItemContext } from '../contexts/AccordionItemContext'; @@ -33,12 +33,8 @@ const AccordionItem: React.FC = ({ dir } = useContext(AccordionContext); - const [isOpen, setIsOpen] = useState(activeItems.includes(value)); const isDisabled = rootDisabled || disabled; - - useEffect(() => { - setIsOpen(activeItems.includes(value)); - }, [activeItems, value]); + const isOpen = activeItems.includes(value); const id = useId(); @@ -46,7 +42,6 @@ const AccordionItem: React.FC = ({ {}, disabled: isDisabled }}> void; + onValueChange?: (value: string | string[] | undefined) => void; } const AccordionRoot = ({ @@ -77,7 +77,9 @@ const AccordionRoot = ({ const [activeItems, setActiveItems] = useControllableState( processedValue, processedDefaultValue, - onValueChange + (next) => { + onValueChange?.(actualType === 'single' ? next[0] : next); + } ); // Handle collapsible logic (only applies to single type)