Skip to content

Commit 266d837

Browse files
authored
fix(sidebar): prevent parent nav items from highlighting on child routes (#1110)
## Summary - Fixed a bug where parent nav items (e.g. "Cloud Agent" at `/cloud`, or "Organization" at `/organizations/:id`) appeared highlighted when navigating to child routes like "Sessions" (`/cloud/sessions`). The `SidebarMenuList` component used `pathname.startsWith(item.url + '/')` for active state, which caused any parent route prefix to match. - The fix introduces an `allUrls` prop on `SidebarMenuList` so the more-specific-route check has visibility into **all** sidebar items across all sections — not just the current group. Both `PersonalAppSidebar` and `OrganizationAppSidebar` now compute and pass the full URL list. When a more specific sibling URL matches the current pathname, the less-specific parent item is no longer marked active. - The `allUrls` prop is optional and falls back to section-local items, so existing call sites without it still work correctly. ## Verification - [x] `pnpm typecheck` — passed with no errors - [x] `pnpm run format:changed` — prettier applied, no remaining formatting issues - [x] Manual review confirming cross-section cases are handled (e.g. `/organizations/:id` vs `/organizations/:id/cloud/sessions`) ## Visual Changes N/A ## Reviewer Notes - Three files changed: `SidebarMenuList.tsx` (active state logic), `PersonalAppSidebar.tsx` and `OrganizationAppSidebar.tsx` (pass `allUrls` prop). - The `hasMoreSpecificMatch` check is short-circuited behind `matchesPrefix` so it only runs when the item already prefix-matched, keeping the common case efficient. - The `allUrls` array is memoized in both sidebar components. --- Built for [Evgeny Shurakov](https://kilo-code.slack.com/archives/C09M96CFZQX/p1773650401612769?thread_ts=1773650249.099499&cid=C09M96CFZQX) by [Kilo for Slack](https://kilo.ai/features/slack-integration)
2 parents 516be5f + 6f07bf8 commit 266d837

3 files changed

Lines changed: 41 additions & 9 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: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,33 @@ 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>
3137
<SidebarGroupLabel className="text-muted-foreground font-medium">{label}</SidebarGroupLabel>
3238
<SidebarGroupContent>
3339
<SidebarMenu>
3440
{items.map(item => {
35-
const isActive = pathname === item.url || pathname.startsWith(item.url + '/');
41+
const matchesPrefix = pathname === item.url || pathname.startsWith(item.url + '/');
42+
const hasMoreSpecificMatch =
43+
matchesPrefix &&
44+
urlsToCheck.some(
45+
url =>
46+
url !== item.url &&
47+
url.length > item.url.length &&
48+
(pathname === url || pathname.startsWith(url + '/'))
49+
);
50+
const isActive = matchesPrefix && !hasMoreSpecificMatch;
3651
return (
3752
<SidebarMenuItem key={item.title}>
3853
<SidebarMenuButton asChild>

0 commit comments

Comments
 (0)