Skip to content

Commit 992bbae

Browse files
authored
refactor: tweaks around forwardRef (#2707)
1 parent b04aff2 commit 992bbae

3 files changed

Lines changed: 135 additions & 114 deletions

File tree

.changeset/great-actors-float.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
"@frontify/fondue-components": patch
3+
"@frontify/fondue": patch
4+
---
5+
6+
refactor: remove unused `forwardRef` from `Tabs.Tab` and add `forwardRef` in `Accordion`

packages/components/src/components/Accordion/Accordion.tsx

Lines changed: 128 additions & 113 deletions
Original file line numberDiff line numberDiff line change
@@ -51,32 +51,38 @@ export type AccordionRootProps = {
5151
onValueChange?: (value: string[]) => void;
5252
};
5353

54-
export const AccordionRoot = ({
55-
'data-test-id': dataTestId = 'fondue-accordion',
56-
border = true,
57-
children,
58-
defaultValue,
59-
disabled,
60-
value,
61-
padding = 'large',
62-
onValueChange,
63-
}: AccordionRootProps) => {
64-
return (
65-
<RadixAccordion.Root
66-
className={styles.root}
67-
data-test-id={dataTestId}
68-
defaultValue={defaultValue}
69-
disabled={disabled}
70-
type="multiple"
71-
value={value}
72-
data-border={border}
73-
data-accordion-padding={padding}
74-
onValueChange={onValueChange}
75-
>
76-
{children}
77-
</RadixAccordion.Root>
78-
);
79-
};
54+
export const AccordionRoot = forwardRef<HTMLDivElement, AccordionRootProps>(
55+
(
56+
{
57+
'data-test-id': dataTestId = 'fondue-accordion',
58+
border = true,
59+
children,
60+
defaultValue,
61+
disabled,
62+
value,
63+
padding = 'large',
64+
onValueChange,
65+
}: AccordionRootProps,
66+
ref: ForwardedRef<HTMLDivElement>,
67+
) => {
68+
return (
69+
<RadixAccordion.Root
70+
ref={ref}
71+
className={styles.root}
72+
data-test-id={dataTestId}
73+
defaultValue={defaultValue}
74+
disabled={disabled}
75+
type="multiple"
76+
value={value}
77+
data-border={border}
78+
data-accordion-padding={padding}
79+
onValueChange={onValueChange}
80+
>
81+
{children}
82+
</RadixAccordion.Root>
83+
);
84+
},
85+
);
8086
AccordionRoot.displayName = 'Accordion.Root';
8187

8288
export type AccordionItemProps = {
@@ -97,29 +103,30 @@ export type AccordionItemProps = {
97103
value: string;
98104
};
99105

100-
export const AccordionItem = ({
101-
'data-test-id': dataTestId = 'fondue-accordion-item',
102-
children,
103-
disabled,
104-
value,
105-
}: AccordionItemProps) => {
106-
return (
107-
<RadixAccordion.Item
108-
className={styles.accordionItem}
109-
value={value}
110-
onPointerDown={(event) => {
111-
event.currentTarget.dataset.showFocusRing = 'false';
112-
}}
113-
onBlur={(event) => {
114-
event.currentTarget.dataset.showFocusRing = 'true';
115-
}}
116-
disabled={disabled}
117-
data-test-id={dataTestId}
118-
>
119-
{children}
120-
</RadixAccordion.Item>
121-
);
122-
};
106+
export const AccordionItem = forwardRef<HTMLDivElement, AccordionItemProps>(
107+
(
108+
{ 'data-test-id': dataTestId = 'fondue-accordion-item', children, disabled, value }: AccordionItemProps,
109+
ref: ForwardedRef<HTMLDivElement>,
110+
) => {
111+
return (
112+
<RadixAccordion.Item
113+
ref={ref}
114+
className={styles.accordionItem}
115+
value={value}
116+
onPointerDown={(event) => {
117+
event.currentTarget.dataset.showFocusRing = 'false';
118+
}}
119+
onBlur={(event) => {
120+
event.currentTarget.dataset.showFocusRing = 'true';
121+
}}
122+
disabled={disabled}
123+
data-test-id={dataTestId}
124+
>
125+
{children}
126+
</RadixAccordion.Item>
127+
);
128+
},
129+
);
123130
AccordionItem.displayName = 'Accordion.Item';
124131

125132
export type AccordionHeaderProps = {
@@ -139,38 +146,38 @@ export type AccordionHeaderProps = {
139146
children?: ReactNode;
140147
};
141148

142-
export const AccordionHeader = ({
143-
'data-test-id': dataTestId = 'fondue-accordion-header',
144-
asChild,
145-
onClick,
146-
children,
147-
}: AccordionHeaderProps) => {
148-
const { slots, triggerContent } = useMemo(
149-
() =>
150-
Children.toArray(children).reduce<{ slots: ReactNode[]; triggerContent: ReactNode[] }>(
151-
(acc, child) => {
152-
if (isValidElement<AccordionSlotProps>(child) && child.type === ForwardedRefAccordionSlot) {
153-
acc.slots.push(child);
154-
} else {
155-
acc.triggerContent.push(child);
156-
}
157-
return acc;
158-
},
159-
{ slots: [], triggerContent: [] },
160-
),
161-
[children],
162-
);
163-
164-
return (
165-
<RadixAccordion.Header asChild={asChild} className={styles.accordionHeader} onClick={onClick}>
166-
<RadixAccordion.Trigger className={styles.accordionTrigger} data-test-id={dataTestId}>
167-
<div className={styles.accordionTriggerContent}>{triggerContent}</div>
168-
<IconCaretDown className={styles.accordionCaret} size="16" />
169-
</RadixAccordion.Trigger>
170-
{slots}
171-
</RadixAccordion.Header>
172-
);
173-
};
149+
export const AccordionHeader = forwardRef<HTMLHeadingElement, AccordionHeaderProps>(
150+
(
151+
{ 'data-test-id': dataTestId = 'fondue-accordion-header', asChild, onClick, children }: AccordionHeaderProps,
152+
ref: ForwardedRef<HTMLHeadingElement>,
153+
) => {
154+
const { slots, triggerContent } = useMemo(
155+
() =>
156+
Children.toArray(children).reduce<{ slots: ReactNode[]; triggerContent: ReactNode[] }>(
157+
(acc, child) => {
158+
if (isValidElement<AccordionSlotProps>(child) && child.type === ForwardedRefAccordionSlot) {
159+
acc.slots.push(child);
160+
} else {
161+
acc.triggerContent.push(child);
162+
}
163+
return acc;
164+
},
165+
{ slots: [], triggerContent: [] },
166+
),
167+
[children],
168+
);
169+
170+
return (
171+
<RadixAccordion.Header ref={ref} asChild={asChild} className={styles.accordionHeader} onClick={onClick}>
172+
<RadixAccordion.Trigger className={styles.accordionTrigger} data-test-id={dataTestId}>
173+
<div className={styles.accordionTriggerContent}>{triggerContent}</div>
174+
<IconCaretDown className={styles.accordionCaret} size="16" />
175+
</RadixAccordion.Trigger>
176+
{slots}
177+
</RadixAccordion.Header>
178+
);
179+
},
180+
);
174181
AccordionHeader.displayName = 'Accordion.Header';
175182

176183
type AccordionContentProps = {
@@ -194,27 +201,33 @@ type AccordionContentProps = {
194201
padding?: AccordionPadding;
195202
};
196203

197-
export const AccordionContent = ({
198-
'data-test-id': dataTestId = 'collapsible-wrap',
199-
children,
200-
divider = false,
201-
onClick,
202-
padding,
203-
}: AccordionContentProps) => {
204-
return (
205-
<RadixAccordion.Content
206-
className={styles.accordionContent}
207-
onClick={onClick}
208-
data-test-id={dataTestId}
209-
data-item-padding={padding}
210-
data-item-divider={divider}
211-
>
212-
<div className={styles.accordionContentText} data-test-id={`inner-${dataTestId}`}>
213-
{children}
214-
</div>
215-
</RadixAccordion.Content>
216-
);
217-
};
204+
export const AccordionContent = forwardRef<HTMLDivElement, AccordionContentProps>(
205+
(
206+
{
207+
'data-test-id': dataTestId = 'collapsible-wrap',
208+
children,
209+
divider = false,
210+
onClick,
211+
padding,
212+
}: AccordionContentProps,
213+
ref: ForwardedRef<HTMLDivElement>,
214+
) => {
215+
return (
216+
<RadixAccordion.Content
217+
ref={ref}
218+
className={styles.accordionContent}
219+
onClick={onClick}
220+
data-test-id={dataTestId}
221+
data-item-padding={padding}
222+
data-item-divider={divider}
223+
>
224+
<div className={styles.accordionContentText} data-test-id={`inner-${dataTestId}`}>
225+
{children}
226+
</div>
227+
</RadixAccordion.Content>
228+
);
229+
},
230+
);
218231
AccordionContent.displayName = 'Accordion.Content';
219232

220233
export type AccordionSlotProps = {
@@ -223,19 +236,21 @@ export type AccordionSlotProps = {
223236
'data-test-id'?: string;
224237
};
225238

226-
export const AccordionSlot = (
227-
{ children, name, 'data-test-id': dataTestId = 'fondue-accordion-slot' }: AccordionSlotProps,
228-
ref: ForwardedRef<HTMLDivElement>,
229-
) => {
230-
return (
231-
<div data-name={name} className={styles.accordionSlot} data-test-id={dataTestId} ref={ref}>
232-
{children}
233-
</div>
234-
);
235-
};
239+
export const AccordionSlot = forwardRef<HTMLDivElement, AccordionSlotProps>(
240+
(
241+
{ children, name, 'data-test-id': dataTestId = 'fondue-accordion-slot' }: AccordionSlotProps,
242+
ref: ForwardedRef<HTMLDivElement>,
243+
) => {
244+
return (
245+
<div data-name={name} className={styles.accordionSlot} data-test-id={dataTestId} ref={ref}>
246+
{children}
247+
</div>
248+
);
249+
},
250+
);
236251
AccordionSlot.displayName = 'Accordion.Slot';
237252

238-
const ForwardedRefAccordionSlot = forwardRef<HTMLDivElement, AccordionSlotProps>(AccordionSlot);
253+
const ForwardedRefAccordionSlot = AccordionSlot;
239254

240255
/**
241256
* @deprecated Use `Accordion.Header` instead.

packages/components/src/components/Tabs/Tabs.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ TabsContent.displayName = 'Tabs.Content';
240240

241241
export const Tabs = {
242242
Root: forwardRef<HTMLDivElement, TabsRootProps>(TabsRoot),
243-
Tab: forwardRef<HTMLDivElement, TabsTabProps>(TabsTab),
243+
Tab: TabsTab,
244244
Trigger: forwardRef<HTMLButtonElement, TabsTriggerProps>(TabsTrigger),
245245
Content: forwardRef<HTMLDivElement, TabsContentProps>(TabsContent),
246246
};

0 commit comments

Comments
 (0)