Skip to content

Commit b62e2d8

Browse files
luannmoreiraclaude
andcommitted
fix(ui-react): address review comments on PR #6233
- Fix min-h-screen → h-full on <aside> in SidebarShell so the sidebar fills its flex parent (h-screen → flex-1 min-h-0) correctly instead of forcing 100vh regardless of containment - Remove dead code from useSidebarLayout: hover-expand state/handlers (expanded, pinned, hoverTimer, handleExpand, handleCollapse, handleToggle) and corresponding hover/focus/toggle handlers are no longer used since desktop sidebar is always open; simplify isOpen = isDesktop - Remove pinned prop from Sidebar, AdminSidebar, and SidebarShell; simplify toggle button to a static close action Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 9baaa51 commit b62e2d8

6 files changed

Lines changed: 9 additions & 51 deletions

File tree

ui-react/apps/console/src/components/layout/AdminLayout.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@ import { useSidebarLayout } from "@/hooks/useSidebarLayout";
66

77
export default function AdminLayout() {
88
const { pathname } = useLocation();
9-
const { isOpen, pinned, isDesktop, drawerOpen, handlers } = useSidebarLayout();
9+
const { isOpen, isDesktop, drawerOpen, handlers } = useSidebarLayout();
1010

1111
return (
1212
<div className="flex flex-col h-screen bg-background">
1313
<div className="flex flex-1 min-h-0 overflow-hidden">
1414
{isDesktop ? (
15-
<AdminSidebar expanded={isOpen} pinned={pinned} />
15+
<AdminSidebar expanded={isOpen} />
1616
) : (
1717
<SidebarMobileDrawer
1818
open={drawerOpen}
@@ -21,7 +21,6 @@ export default function AdminLayout() {
2121
>
2222
<AdminSidebar
2323
expanded
24-
pinned={false}
2524
onToggle={handlers.closeDrawer}
2625
onClose={handlers.closeDrawer}
2726
/>

ui-react/apps/console/src/components/layout/AdminSidebar.tsx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,12 +168,10 @@ function NavGroupItem({
168168

169169
export default function AdminSidebar({
170170
expanded,
171-
pinned,
172171
onToggle,
173172
onClose,
174173
}: {
175174
expanded: boolean;
176-
pinned: boolean;
177175
onToggle?: () => void;
178176
onClose?: () => void;
179177
}) {
@@ -200,7 +198,6 @@ export default function AdminSidebar({
200198
return (
201199
<SidebarShell
202200
expanded={expanded}
203-
pinned={pinned}
204201
onToggle={onToggle}
205202
onClose={onClose}
206203
ariaLabel="Admin navigation"

ui-react/apps/console/src/components/layout/AppLayout.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ export default function AppLayout() {
1515
const hasVisibleTerminal = useTerminalStore((s) =>
1616
s.sessions.some((t) => t.state !== "minimized"),
1717
);
18-
const { isOpen, pinned, isDesktop, drawerOpen, handlers } = useSidebarLayout();
18+
const { isOpen, isDesktop, drawerOpen, handlers } = useSidebarLayout();
1919

2020
const showSidebar = namespaces.length > 0;
2121
const sidebarOffset =
@@ -28,7 +28,7 @@ export default function AppLayout() {
2828
<ConnectivityBanner />
2929
<div className="flex flex-1 min-h-0">
3030
{showSidebar && isDesktop && (
31-
<Sidebar expanded={isOpen} pinned={pinned} />
31+
<Sidebar expanded={isOpen} />
3232
)}
3333
{showSidebar && !isDesktop && (
3434
<SidebarMobileDrawer
@@ -38,7 +38,6 @@ export default function AppLayout() {
3838
>
3939
<Sidebar
4040
expanded
41-
pinned={false}
4241
onToggle={handlers.closeDrawer}
4342
onClose={handlers.closeDrawer}
4443
/>

ui-react/apps/console/src/components/layout/Sidebar.tsx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,10 @@ function ProBadge() {
9898

9999
export default function Sidebar({
100100
expanded,
101-
pinned,
102101
onToggle,
103102
onClose,
104103
}: {
105104
expanded: boolean;
106-
pinned: boolean;
107105
onToggle?: () => void;
108106
onClose?: () => void;
109107
}) {
@@ -120,7 +118,6 @@ export default function Sidebar({
120118
return (
121119
<SidebarShell
122120
expanded={expanded}
123-
pinned={pinned}
124121
onToggle={onToggle}
125122
onClose={onClose}
126123
hidden={isFullscreen}

ui-react/apps/console/src/components/layout/SidebarShell.tsx

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,6 @@ export function SidebarMobileDrawer({
108108

109109
interface SidebarShellProps {
110110
expanded: boolean;
111-
pinned: boolean;
112111
onToggle?: () => void;
113112
onClose?: () => void;
114113
hidden?: boolean;
@@ -120,7 +119,6 @@ interface SidebarShellProps {
120119

121120
export default function SidebarShell({
122121
expanded,
123-
pinned,
124122
onToggle,
125123
onClose,
126124
hidden,
@@ -131,7 +129,7 @@ export default function SidebarShell({
131129
}: SidebarShellProps) {
132130
return (
133131
<aside
134-
className={`bg-surface border-r border-border flex flex-col min-h-screen shrink-0 transition-all duration-200 ease-in-out overflow-hidden ${
132+
className={`bg-surface border-r border-border flex flex-col h-full shrink-0 transition-all duration-200 ease-in-out overflow-hidden ${
135133
hidden ? "w-0 opacity-0" : expanded ? "w-[220px]" : "w-[60px]"
136134
}`}
137135
>
@@ -177,14 +175,10 @@ export default function SidebarShell({
177175
type="button"
178176
onClick={onToggle}
179177
tabIndex={expanded ? 0 : -1}
180-
aria-label={pinned ? "Unpin sidebar" : "Pin sidebar"}
181-
title={pinned ? "Unpin sidebar" : "Pin sidebar open"}
182-
className={`p-1 rounded-md transition-all duration-200 ${
178+
aria-label="Close sidebar"
179+
title="Close sidebar"
180+
className={`p-1 rounded-md transition-all duration-200 text-text-muted hover:text-text-primary hover:bg-hover-subtle ${
183181
expanded ? "opacity-100" : "opacity-0"
184-
} ${
185-
pinned
186-
? "text-primary bg-primary/10"
187-
: "text-text-muted hover:text-text-primary hover:bg-hover-subtle"
188182
}`}
189183
>
190184
<ChevronLeftIcon

ui-react/apps/console/src/hooks/useSidebarLayout.ts

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
import {
22
useState,
3-
useRef,
4-
useEffect,
53
useSyncExternalStore,
64
} from "react";
75

@@ -24,8 +22,6 @@ function getIsDesktopServer() {
2422
}
2523

2624
export function useSidebarLayout() {
27-
const [expanded, setExpanded] = useState(false);
28-
const [pinned, setPinned] = useState(false);
2925
const [drawerOpen, setDrawerOpen] = useState(false);
3026

3127
const isDesktop = useSyncExternalStore(
@@ -34,43 +30,19 @@ export function useSidebarLayout() {
3430
getIsDesktopServer,
3531
);
3632

37-
const hoverTimer = useRef<ReturnType<typeof setTimeout>>(undefined);
38-
39-
const isOpen = isDesktop || expanded || pinned;
33+
const isOpen = isDesktop;
4034

4135
const openDrawer = () => setDrawerOpen(true);
4236
const closeDrawer = () => setDrawerOpen(false);
4337
const toggleDrawer = () => setDrawerOpen((prev) => !prev);
4438

45-
// Clean up hover timer on unmount
46-
useEffect(() => () => clearTimeout(hoverTimer.current), []);
47-
48-
const handleExpand = () => {
49-
clearTimeout(hoverTimer.current);
50-
hoverTimer.current = setTimeout(() => setExpanded(true), 75);
51-
};
52-
53-
const handleCollapse = () => {
54-
clearTimeout(hoverTimer.current);
55-
hoverTimer.current = setTimeout(() => setExpanded(false), 150);
56-
};
57-
58-
const handleToggle = () => { setPinned((prev) => !prev); };
59-
6039
const handleDrawerKeyDown = (e: React.KeyboardEvent) => { if (e.key === "Escape") closeDrawer(); };
6140

6241
return {
63-
expanded,
64-
pinned,
6542
isOpen,
6643
isDesktop,
6744
drawerOpen,
6845
handlers: {
69-
onMouseEnter: handleExpand,
70-
onMouseLeave: handleCollapse,
71-
onFocus: handleExpand,
72-
onBlur: handleCollapse,
73-
onToggle: handleToggle,
7446
openDrawer,
7547
closeDrawer,
7648
toggleDrawer,

0 commit comments

Comments
 (0)