Skip to content

Commit 6f07bf8

Browse files
fix(sidebar): check all sidebar URLs for active state, not just current section
The previous fix only checked sibling items within the same SidebarMenuList group. This meant cross-section conflicts (e.g. Organization at /organizations/:id in Dashboard vs Cloud Agent at /organizations/:id/cloud in Cloud) were not caught. Add an allUrls prop to SidebarMenuList so the more-specific-route check has visibility into all sidebar items across all sections. Both PersonalAppSidebar and OrganizationAppSidebar now compute and pass the full URL list. Falls back to section-local items when allUrls is omitted.
1 parent bfbc8a3 commit 6f07bf8

3 files changed

Lines changed: 36 additions & 13 deletions

File tree

src/app/(app)/components/OrganizationAppSidebar.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -246,6 +246,11 @@ export default function OrganizationAppSidebar({
246246
: []),
247247
];
248248

249+
const allUrls = useMemo(
250+
() => [...dashboardItems, ...cloudItems, ...accountItems].map(i => i.url),
251+
[dashboardItems, cloudItems, accountItems]
252+
);
253+
249254
// Determine if we should show the OrganizationSwitcher
250255
// Hide it when an admin user is viewing an organization they're not a member of
251256
const shouldShowOrganizationSwitcher = !user?.is_admin || currentOrg;
@@ -266,9 +271,13 @@ export default function OrganizationAppSidebar({
266271
</SidebarHeader>
267272

268273
<SidebarContent>
269-
<SidebarMenuList label="Dashboard" items={dashboardItems} />
270-
{cloudItems.length > 0 && <SidebarMenuList label="Cloud" items={cloudItems} />}
271-
{accountItems.length > 0 && <SidebarMenuList label="Account" items={accountItems} />}
274+
<SidebarMenuList label="Dashboard" items={dashboardItems} allUrls={allUrls} />
275+
{cloudItems.length > 0 && (
276+
<SidebarMenuList label="Cloud" items={cloudItems} allUrls={allUrls} />
277+
)}
278+
{accountItems.length > 0 && (
279+
<SidebarMenuList label="Account" items={accountItems} allUrls={allUrls} />
280+
)}
272281
</SidebarContent>
273282

274283
<SidebarUserFooter user={user} isLoading={isLoading} />

src/app/(app)/components/PersonalAppSidebar.tsx

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
Webhook,
2626
Factory,
2727
} from 'lucide-react';
28+
import { useMemo } from 'react';
2829
import HeaderLogo from '@/components/HeaderLogo';
2930
import OrganizationSwitcher from './OrganizationSwitcher';
3031
import SidebarMenuList from './SidebarMenuList';
@@ -202,6 +203,11 @@ export default function PersonalAppSidebar(props: React.ComponentProps<typeof Si
202203
},
203204
];
204205

206+
const allUrls = useMemo(
207+
() => [...dashboardItems, ...cloudItems, ...accountItems, ...startItems].map(i => i.url),
208+
[dashboardItems, cloudItems, accountItems, startItems]
209+
);
210+
205211
return (
206212
<Sidebar {...props}>
207213
<SidebarHeader className="p-4">
@@ -216,10 +222,12 @@ export default function PersonalAppSidebar(props: React.ComponentProps<typeof Si
216222
</SidebarHeader>
217223

218224
<SidebarContent>
219-
<SidebarMenuList label="Dashboard" items={dashboardItems} />
220-
{cloudItems.length > 0 && <SidebarMenuList label="Cloud" items={cloudItems} />}
221-
<SidebarMenuList label="Account" items={accountItems} />
222-
<SidebarMenuList label="Start" items={startItems} />
225+
<SidebarMenuList label="Dashboard" items={dashboardItems} allUrls={allUrls} />
226+
{cloudItems.length > 0 && (
227+
<SidebarMenuList label="Cloud" items={cloudItems} allUrls={allUrls} />
228+
)}
229+
<SidebarMenuList label="Account" items={accountItems} allUrls={allUrls} />
230+
<SidebarMenuList label="Start" items={startItems} allUrls={allUrls} />
223231
</SidebarContent>
224232

225233
<SidebarUserFooter user={user} isLoading={isLoading} />

src/app/(app)/components/SidebarMenuList.tsx

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,16 @@ type MenuItem = {
2121
type SidebarMenuListProps = {
2222
items: MenuItem[];
2323
label?: string;
24+
allUrls?: string[];
2425
};
2526

26-
export default function SidebarMenuList({ items, label = 'Dashboard' }: SidebarMenuListProps) {
27+
export default function SidebarMenuList({
28+
items,
29+
label = 'Dashboard',
30+
allUrls,
31+
}: SidebarMenuListProps) {
2732
const pathname = usePathname();
33+
const urlsToCheck = allUrls ?? items.map(i => i.url);
2834

2935
return (
3036
<SidebarGroup>
@@ -35,11 +41,11 @@ export default function SidebarMenuList({ items, label = 'Dashboard' }: SidebarM
3541
const matchesPrefix = pathname === item.url || pathname.startsWith(item.url + '/');
3642
const hasMoreSpecificMatch =
3743
matchesPrefix &&
38-
items.some(
39-
other =>
40-
other.url !== item.url &&
41-
other.url.length > item.url.length &&
42-
(pathname === other.url || pathname.startsWith(other.url + '/'))
44+
urlsToCheck.some(
45+
url =>
46+
url !== item.url &&
47+
url.length > item.url.length &&
48+
(pathname === url || pathname.startsWith(url + '/'))
4349
);
4450
const isActive = matchesPrefix && !hasMoreSpecificMatch;
4551
return (

0 commit comments

Comments
 (0)