Skip to content

Commit 0650836

Browse files
trepechovclaude
andcommitted
fix: address PR review issues in Sidebar
- Fix Rules of Hooks violation: move useEffect above early return guard - Move bottomQuickActions to module scope (no component deps) - Extract duplicate profile dropdown JSX into profileMenuItems variable - Add aria-label to collapsed avatar button for screen reader support Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 59b5bb9 commit 0650836

1 file changed

Lines changed: 45 additions & 106 deletions

File tree

apps/webapp/app/components/Sidebar.tsx

Lines changed: 45 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,11 @@ const navItems = [
3535
{ href: "/data-sources", label: "Data Sources", icon: dataSourcesIconPath },
3636
];
3737

38+
const bottomQuickActions = [
39+
{ id: "workspaces", href: "/workspaces", label: "Workspaces", iconPath: workspacesIconPath },
40+
{ id: "worker-logs", href: "/worker-logs", label: "Worker Logs", iconPath: workerLogsIconPath },
41+
];
42+
3843
const getGravatarUrl = (email: string, size: number = 80) => {
3944
const hash = md5(email.toLowerCase().trim());
4045
return `https://www.gravatar.com/avatar/${hash}?s=${size}&d=identicon`;
@@ -58,29 +63,8 @@ export function Sidebar() {
5863
logoutMutation.mutate();
5964
};
6065

61-
const bottomQuickActions = [
62-
{
63-
id: "workspaces",
64-
kind: "link" as const,
65-
href: "/workspaces",
66-
label: "Workspaces",
67-
iconPath: workspacesIconPath,
68-
},
69-
{
70-
id: "worker-logs",
71-
kind: "link" as const,
72-
href: "/worker-logs",
73-
label: "Worker Logs",
74-
iconPath: workerLogsIconPath,
75-
},
76-
];
77-
7866
const isActive = (path: string) => pathname === path;
7967

80-
if (!userData?.user) return null;
81-
82-
const user = userData.user;
83-
8468
useEffect(() => {
8569
if (!isProfileMenuOpen) return;
8670

@@ -103,6 +87,43 @@ export function Sidebar() {
10387
};
10488
}, [isProfileMenuOpen]);
10589

90+
if (!userData?.user) return null;
91+
92+
const user = userData.user;
93+
94+
const profileMenuItems = (
95+
<>
96+
<Link
97+
role="menuitem"
98+
href="/profile"
99+
onClick={() => setIsProfileMenuOpen(false)}
100+
className={`flex items-center gap-2 px-3 py-2 rounded-lg text-sm font-mono transition-colors ${
101+
isActive("/profile") ? "bg-primary/10 text-primary" : "hover:bg-base-200"
102+
}`}
103+
>
104+
<svg className="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
105+
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={profileSettingsIconPath} />
106+
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={profileSettingsUserIconPath} />
107+
</svg>
108+
<span>Profile Settings</span>
109+
</Link>
110+
<button
111+
role="menuitem"
112+
type="button"
113+
onClick={() => {
114+
setIsProfileMenuOpen(false);
115+
handleLogout();
116+
}}
117+
className="w-full flex items-center gap-2 px-3 py-2 rounded-lg text-sm font-mono hover:bg-error/10 hover:text-error transition-colors"
118+
>
119+
<svg className="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
120+
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={logoutIconPath} />
121+
</svg>
122+
<span>Logout</span>
123+
</button>
124+
</>
125+
);
126+
106127
return (
107128
<aside
108129
className={`h-full min-h-screen bg-base-100 border-r border-base-300 transition-all duration-300 ease-in-out flex flex-col pt-16
@@ -291,36 +312,7 @@ export function Sidebar() {
291312
role="menu"
292313
className="absolute bottom-full left-0 mb-2 w-full rounded-lg border border-base-300 bg-base-100 shadow-xl p-1 z-50"
293314
>
294-
<Link
295-
role="menuitem"
296-
href="/profile"
297-
onClick={() => setIsProfileMenuOpen(false)}
298-
className={`flex items-center gap-2 px-3 py-2 rounded-lg text-sm font-mono transition-colors ${
299-
isActive("/profile")
300-
? "bg-primary/10 text-primary"
301-
: "hover:bg-base-200"
302-
}`}
303-
>
304-
<svg className="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
305-
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={profileSettingsIconPath} />
306-
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={profileSettingsUserIconPath} />
307-
</svg>
308-
<span>Profile Settings</span>
309-
</Link>
310-
<button
311-
role="menuitem"
312-
type="button"
313-
onClick={() => {
314-
setIsProfileMenuOpen(false);
315-
handleLogout();
316-
}}
317-
className="w-full flex items-center gap-2 px-3 py-2 rounded-lg text-sm font-mono hover:bg-error/10 hover:text-error transition-colors"
318-
>
319-
<svg className="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
320-
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={logoutIconPath} />
321-
</svg>
322-
<span>Logout</span>
323-
</button>
315+
{profileMenuItems}
324316
</div>
325317
)}
326318
</div>
@@ -353,6 +345,7 @@ export function Sidebar() {
353345
onClick={() => setIsProfileMenuOpen((v) => !v)}
354346
className="rounded-full ring-2 ring-primary/20 hover:ring-primary/40 transition-all"
355347
title={user.username}
348+
aria-label={user.username}
356349
aria-haspopup="menu"
357350
aria-expanded={isProfileMenuOpen}
358351
>
@@ -368,61 +361,7 @@ export function Sidebar() {
368361
role="menu"
369362
className="absolute bottom-full left-1/2 -translate-x-1/2 mb-2 w-52 rounded-lg border border-base-300 bg-base-100 shadow-xl p-1 z-50"
370363
>
371-
<Link
372-
role="menuitem"
373-
href="/profile"
374-
onClick={() => setIsProfileMenuOpen(false)}
375-
className={`flex items-center gap-2 px-3 py-2 rounded-lg text-sm font-mono transition-colors ${
376-
isActive("/profile")
377-
? "bg-primary/10 text-primary"
378-
: "hover:bg-base-200"
379-
}`}
380-
>
381-
<svg
382-
className="w-4 h-4"
383-
fill="none"
384-
stroke="currentColor"
385-
viewBox="0 0 24 24"
386-
>
387-
<path
388-
strokeLinecap="round"
389-
strokeLinejoin="round"
390-
strokeWidth={2}
391-
d={profileSettingsIconPath}
392-
/>
393-
<path
394-
strokeLinecap="round"
395-
strokeLinejoin="round"
396-
strokeWidth={2}
397-
d={profileSettingsUserIconPath}
398-
/>
399-
</svg>
400-
<span>Profile Settings</span>
401-
</Link>
402-
<button
403-
role="menuitem"
404-
type="button"
405-
onClick={() => {
406-
setIsProfileMenuOpen(false);
407-
handleLogout();
408-
}}
409-
className="w-full flex items-center gap-2 px-3 py-2 rounded-lg text-sm font-mono hover:bg-error/10 hover:text-error transition-colors"
410-
>
411-
<svg
412-
className="w-4 h-4"
413-
fill="none"
414-
stroke="currentColor"
415-
viewBox="0 0 24 24"
416-
>
417-
<path
418-
strokeLinecap="round"
419-
strokeLinejoin="round"
420-
strokeWidth={2}
421-
d={logoutIconPath}
422-
/>
423-
</svg>
424-
<span>Logout</span>
425-
</button>
364+
{profileMenuItems}
426365
</div>
427366
)}
428367
</div>

0 commit comments

Comments
 (0)