From caef89a7aa7d6aafe9133dfbc6ce1814d67e0b57 Mon Sep 17 00:00:00 2001
From: Tam Nhu Tran <kaitran.ntt@gmail.com>
Date: Sat, 25 Apr 2026 12:43:41 -0400
Subject: [PATCH] fix(ui): address PR-Agent round 2 review feedback

MonitorLayout:
- Wrap ScrollArea in an explicit flex-col container with min-h-0. The
  previous version relied on the parent (PageShell) being flex-col for
  flex-1 to compute a definite height; that's fragile. The wrapper now
  establishes its own flex column so the ScrollArea reliably scrolls
  whether or not the caller's wrapper is flex.

ConfigLayout (MobileTabs):
- Derive active tab during render with fallback to first available, same
  pattern applied to JsonPane in the previous round. Parent toggling
  `left` or `json` props could change the available tabs and leave
  the controlled Tabs value pointing at a tab id that no longer existed,
  rendering an empty pane on mobile until the user manually switched.
---
 .../components/config-layout/config-layout.tsx  |  9 +++++++--
 .../monitor-layout/monitor-layout.tsx           | 17 +++++++++++------
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/ui/src/components/config-layout/config-layout.tsx b/ui/src/components/config-layout/config-layout.tsx
index 3534dccb..c48ccf3d 100644
--- a/ui/src/components/config-layout/config-layout.tsx
+++ b/ui/src/components/config-layout/config-layout.tsx
@@ -63,11 +63,16 @@ function MobileTabs({
     { id: 'form', label: 'Configure', node: form },
     json && { id: 'json', label: 'JSON', node: json },
   ].filter(Boolean) as { id: string; label: string; node: ReactNode }[];
-  const [active, setActive] = useState(tabs[0]?.id ?? 'form');
+  const [selected, setSelected] = useState(tabs[0]?.id ?? 'form');
+
+  // Derive the effective active tab during render so a parent toggling `left`
+  // or `json` (which changes the available tabs) cannot leave us pointing at
+  // an id that no longer exists. Falls back to the first available tab.
+  const active = tabs.some((t) => t.id === selected) ? selected : (tabs[0]?.id ?? 'form');
 
   return (
     <div className={cn('flex min-h-0 flex-1 flex-col gap-3 p-3 lg:hidden', className)}>
-      <Tabs value={active} onValueChange={setActive}>
+      <Tabs value={active} onValueChange={setSelected}>
         <TabsList className="w-full">
           {tabs.map((t) => (
             <TabsTrigger key={t.id} value={t.id} className="flex-1">
diff --git a/ui/src/components/monitor-layout/monitor-layout.tsx b/ui/src/components/monitor-layout/monitor-layout.tsx
index 1e9d6771..71bc84eb 100644
--- a/ui/src/components/monitor-layout/monitor-layout.tsx
+++ b/ui/src/components/monitor-layout/monitor-layout.tsx
@@ -25,12 +25,17 @@ interface MonitorLayoutProps {
  *   </PageShell>
  */
 export function MonitorLayout({ kpis, children, className }: MonitorLayoutProps) {
+  // Wrapper establishes its own flex column with min-h-0 so the ScrollArea
+  // gets a definite height and can scroll its content. Without this we'd be
+  // implicitly relying on PageShell being flex-col, which is fragile.
   return (
-    <ScrollArea className={cn('flex-1', className)}>
-      <div className="space-y-4 p-4 sm:p-6">
-        {kpis}
-        {children}
-      </div>
-    </ScrollArea>
+    <div className={cn('flex min-h-0 flex-1 flex-col', className)}>
+      <ScrollArea className="flex-1">
+        <div className="space-y-4 p-4 sm:p-6">
+          {kpis}
+          {children}
+        </div>
+      </ScrollArea>
+    </div>
   );
 }