Skip to content

Commit c0f8780

Browse files
jescalanjescalan
committed
Address custom portal review feedback
1 parent f1563ec commit c0f8780

5 files changed

Lines changed: 140 additions & 17 deletions

File tree

‎packages/react/src/utils/__tests__/useCustomMenuItems.test.tsx‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,4 +135,41 @@ describe('useUserButtonCustomMenuItems', () => {
135135
expect(mockOnClick).toHaveBeenCalledTimes(1);
136136
}
137137
});
138+
139+
it('keeps portal identity with the logical menu item when inserting before it', () => {
140+
const firstItem = (
141+
<MenuAction
142+
key='first'
143+
label='First action'
144+
labelIcon={<div>First icon</div>}
145+
onClick={() => {}}
146+
/>
147+
);
148+
const secondItem = (
149+
<MenuAction
150+
key='second'
151+
label='Second action'
152+
labelIcon={<div>Second icon</div>}
153+
onClick={() => {}}
154+
/>
155+
);
156+
const makeChildren = (includeFirstItem: boolean) => (
157+
<MenuItems>{includeFirstItem ? [firstItem, secondItem] : [secondItem]}</MenuItems>
158+
);
159+
160+
const { result, rerender } = renderHook(
161+
({ includeFirstItem }) => useUserButtonCustomMenuItems(makeChildren(includeFirstItem)),
162+
{
163+
initialProps: { includeFirstItem: false },
164+
},
165+
);
166+
167+
const secondItemIconPortal = result.current.customMenuItemsPortals[0];
168+
169+
rerender({ includeFirstItem: true });
170+
171+
expect(result.current.customMenuItems).toHaveLength(2);
172+
expect(result.current.customMenuItemsPortals[0]).not.toBe(secondItemIconPortal);
173+
expect(result.current.customMenuItemsPortals[1]).toBe(secondItemIconPortal);
174+
});
138175
});

‎packages/react/src/utils/__tests__/useCustomPages.test.tsx‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { renderHook } from '@testing-library/react';
2+
import React from 'react';
3+
import { describe, expect, it, vi } from 'vitest';
4+
5+
import { OrganizationProfilePage } from '../../components/uiComponents';
6+
import { useOrganizationProfileCustomPages } from '../useCustomPages';
7+
8+
vi.mock('@clerk/shared', () => ({
9+
logErrorInDevMode: vi.fn(),
10+
}));
11+
12+
describe('useOrganizationProfileCustomPages', () => {
13+
it('keeps portal identity with the logical custom page when inserting before it', () => {
14+
const firstPage = (
15+
<OrganizationProfilePage
16+
key='first'
17+
label='First page'
18+
labelIcon={<div>First icon</div>}
19+
url='first'
20+
>
21+
<div>First content</div>
22+
</OrganizationProfilePage>
23+
);
24+
const secondPage = (
25+
<OrganizationProfilePage
26+
key='second'
27+
label='Second page'
28+
labelIcon={<div>Second icon</div>}
29+
url='second'
30+
>
31+
<div>Second content</div>
32+
</OrganizationProfilePage>
33+
);
34+
const makeChildren = (includeFirstPage: boolean) => (includeFirstPage ? [firstPage, secondPage] : [secondPage]);
35+
36+
const { result, rerender } = renderHook(
37+
({ includeFirstPage }) => useOrganizationProfileCustomPages(makeChildren(includeFirstPage)),
38+
{
39+
initialProps: { includeFirstPage: false },
40+
},
41+
);
42+
43+
const secondPageContentPortal = result.current.customPagesPortals[0];
44+
const secondPageIconPortal = result.current.customPagesPortals[1];
45+
46+
rerender({ includeFirstPage: true });
47+
48+
expect(result.current.customPages).toHaveLength(2);
49+
expect(result.current.customPagesPortals[0]).not.toBe(secondPageContentPortal);
50+
expect(result.current.customPagesPortals[1]).not.toBe(secondPageIconPortal);
51+
expect(result.current.customPagesPortals[2]).toBe(secondPageContentPortal);
52+
expect(result.current.customPagesPortals[3]).toBe(secondPageIconPortal);
53+
});
54+
});

‎packages/react/src/utils/useCustomElementPortal.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,14 +4,14 @@ import { createPortal } from 'react-dom';
44

55
export type UseCustomElementPortalParams = {
66
component: React.ReactNode;
7-
id: number;
7+
id: string | number;
88
};
99

1010
export type UseCustomElementPortalReturn = {
1111
portal: React.ComponentType;
1212
mount: (node: Element) => void;
1313
unmount: () => void;
14-
id: number;
14+
id: string | number;
1515
};
1616

1717
// This function takes a component as prop, and returns functions that mount and unmount

‎packages/react/src/utils/useCustomMenuItems.tsx‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ type UseCustomMenuItemsParams = {
4343
allowForAnyChildren?: boolean;
4444
};
4545

46-
type CustomMenuItemType = UserButtonActionProps | UserButtonLinkProps;
46+
type CustomMenuItemType = (UserButtonActionProps | UserButtonLinkProps) & { portalId?: string };
4747

4848
const useCustomMenuItems = ({
4949
children,
@@ -89,6 +89,7 @@ const useCustomMenuItems = ({
8989
}
9090

9191
const { props } = child as ReactElement;
92+
const childKey = (child as ReactElement).key;
9293

9394
const { label, labelIcon, href, onClick, open } = props;
9495

@@ -106,11 +107,13 @@ const useCustomMenuItems = ({
106107
validChildren.push({
107108
...baseItem,
108109
onClick,
110+
portalId: getCustomMenuItemPortalId('action', props, childKey),
109111
});
110112
} else if (open !== undefined) {
111113
validChildren.push({
112114
...baseItem,
113115
open: open.startsWith('/') ? open : `/${open}`,
116+
portalId: getCustomMenuItemPortalId('action', props, childKey),
114117
});
115118
} else {
116119
// Handle the case where neither onClick nor open is defined
@@ -125,7 +128,7 @@ const useCustomMenuItems = ({
125128

126129
if (isThatComponent(child, MenuLinkComponent)) {
127130
if (isExternalLink(props)) {
128-
validChildren.push({ label, labelIcon, href });
131+
validChildren.push({ label, labelIcon, href, portalId: getCustomMenuItemPortalId('link', props, childKey) });
129132
} else {
130133
logErrorInDevMode(userButtonMenuItemLinkWrongProps);
131134
return;
@@ -138,10 +141,10 @@ const useCustomMenuItems = ({
138141
const customLinkLabelIcons: UseCustomElementPortalParams[] = [];
139142
validChildren.forEach((mi, index) => {
140143
if (isCustomMenuItem(mi)) {
141-
customMenuItemLabelIcons.push({ component: mi.labelIcon, id: index });
144+
customMenuItemLabelIcons.push({ component: mi.labelIcon, id: mi.portalId || index });
142145
}
143146
if (isExternalLink(mi)) {
144-
customLinkLabelIcons.push({ component: mi.labelIcon, id: index });
147+
customLinkLabelIcons.push({ component: mi.labelIcon, id: mi.portalId || index });
145148
}
146149
});
147150

@@ -159,7 +162,7 @@ const useCustomMenuItems = ({
159162
portal: iconPortal,
160163
mount: mountIcon,
161164
unmount: unmountIcon,
162-
} = customMenuItemLabelIconsPortals.find(p => p.id === index) as UseCustomElementPortalReturn;
165+
} = customMenuItemLabelIconsPortals.find(p => p.id === (mi.portalId || index)) as UseCustomElementPortalReturn;
163166
const menuItem: CustomMenuItem = {
164167
label: mi.label,
165168
mountIcon,
@@ -179,7 +182,7 @@ const useCustomMenuItems = ({
179182
portal: iconPortal,
180183
mount: mountIcon,
181184
unmount: unmountIcon,
182-
} = customLinkLabelIconsPortals.find(p => p.id === index) as UseCustomElementPortalReturn;
185+
} = customLinkLabelIconsPortals.find(p => p.id === (mi.portalId || index)) as UseCustomElementPortalReturn;
183186
customMenuItems.push({
184187
label: mi.label,
185188
href: mi.href,
@@ -193,6 +196,19 @@ const useCustomMenuItems = ({
193196
return { customMenuItems, customMenuItemsPortals };
194197
};
195198

199+
const getCustomMenuItemPortalId = (
200+
type: 'action' | 'link',
201+
props: Pick<CustomMenuItemType, 'label'> & { href?: string; open?: string },
202+
key: React.Key | null,
203+
) => {
204+
if (key != null) {
205+
return `${type}:key:${key}`;
206+
}
207+
208+
const target = props.href || props.open || '';
209+
return `${type}:${props.label}:${target}`;
210+
};
211+
196212
const isReorderItem = (childProps: any, validItems: string[]): boolean => {
197213
const { children, label, onClick, labelIcon } = childProps;
198214
return !children && !onClick && !labelIcon && validItems.some(v => v === label);

‎packages/react/src/utils/useCustomPages.tsx‎

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ type UseCustomPagesOptions = {
6464
allowForAnyChildren: boolean;
6565
};
6666

67-
type CustomPageWithIdType = UserProfilePageProps & { children?: React.ReactNode };
67+
type CustomPageWithIdType = UserProfilePageProps & { children?: React.ReactNode; portalId?: string };
6868

6969
/**
7070
* Exclude any children that is used for identifying Custom Pages or Custom Items.
@@ -121,13 +121,21 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp
121121

122122
const { children, label, url, labelIcon } = props;
123123

124+
const childKey = (child as ReactElement).key;
125+
124126
if (isThatComponent(child, PageComponent)) {
125127
if (isReorderItem(props, reorderItemsLabels)) {
126128
// This is a reordering item
127129
validChildren.push({ label });
128130
} else if (isCustomPage(props)) {
129131
// this is a custom page
130-
validChildren.push({ label, labelIcon, children, url });
132+
validChildren.push({
133+
label,
134+
labelIcon,
135+
children,
136+
url,
137+
portalId: getCustomPagePortalId('page', props, childKey),
138+
});
131139
} else {
132140
logErrorInDevMode(customPageWrongProps(componentName));
133141
return;
@@ -137,7 +145,7 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp
137145
if (isThatComponent(child, LinkComponent)) {
138146
if (isExternalLink(props)) {
139147
// This is an external link
140-
validChildren.push({ label, labelIcon, url });
148+
validChildren.push({ label, labelIcon, url, portalId: getCustomPagePortalId('link', props, childKey) });
141149
} else {
142150
logErrorInDevMode(customLinkWrongProps(componentName));
143151
return;
@@ -151,12 +159,12 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp
151159

152160
validChildren.forEach((cp, index) => {
153161
if (isCustomPage(cp)) {
154-
customPageContents.push({ component: cp.children, id: index });
155-
customPageLabelIcons.push({ component: cp.labelIcon, id: index });
162+
customPageContents.push({ component: cp.children, id: cp.portalId || index });
163+
customPageLabelIcons.push({ component: cp.labelIcon, id: cp.portalId || index });
156164
return;
157165
}
158166
if (isExternalLink(cp)) {
159-
customLinkLabelIcons.push({ component: cp.labelIcon, id: index });
167+
customLinkLabelIcons.push({ component: cp.labelIcon, id: cp.portalId || index });
160168
}
161169
});
162170

@@ -177,12 +185,12 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp
177185
portal: contentPortal,
178186
mount,
179187
unmount,
180-
} = customPageContentsPortals.find(p => p.id === index) as UseCustomElementPortalReturn;
188+
} = customPageContentsPortals.find(p => p.id === (cp.portalId || index)) as UseCustomElementPortalReturn;
181189
const {
182190
portal: labelPortal,
183191
mount: mountIcon,
184192
unmount: unmountIcon,
185-
} = customPageLabelIconsPortals.find(p => p.id === index) as UseCustomElementPortalReturn;
193+
} = customPageLabelIconsPortals.find(p => p.id === (cp.portalId || index)) as UseCustomElementPortalReturn;
186194
customPages.push({ label: cp.label, url: cp.url, mount, unmount, mountIcon, unmountIcon });
187195
customPagesPortals.push(contentPortal);
188196
customPagesPortals.push(labelPortal);
@@ -193,7 +201,7 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp
193201
portal: labelPortal,
194202
mount: mountIcon,
195203
unmount: unmountIcon,
196-
} = customLinkLabelIconsPortals.find(p => p.id === index) as UseCustomElementPortalReturn;
204+
} = customLinkLabelIconsPortals.find(p => p.id === (cp.portalId || index)) as UseCustomElementPortalReturn;
197205
customPages.push({ label: cp.label, url: cp.url, mountIcon, unmountIcon });
198206
customPagesPortals.push(labelPortal);
199207
return;
@@ -203,6 +211,14 @@ const useCustomPages = (params: UseCustomPagesParams, options?: UseCustomPagesOp
203211
return { customPages, customPagesPortals };
204212
};
205213

214+
const getCustomPagePortalId = (
215+
type: 'page' | 'link',
216+
props: Pick<CustomPageWithIdType, 'label' | 'url'>,
217+
key: React.Key | null,
218+
) => {
219+
return key == null ? `${type}:${props.label}:${props.url}` : `${type}:key:${key}`;
220+
};
221+
206222
const isReorderItem = (childProps: any, validItems: string[]): boolean => {
207223
const { children, label, url, labelIcon } = childProps;
208224
return !children && !url && !labelIcon && validItems.some(v => v === label);

0 commit comments

Comments
 (0)