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.
This commit is contained in:
Tam Nhu Tran
2026-04-25 12:43:41 -04:00
parent 37de42d463
commit caef89a7aa
2 changed files with 18 additions and 8 deletions
@@ -63,11 +63,16 @@ function MobileTabs({
{ id: 'form', label: 'Configure', node: form }, { id: 'form', label: 'Configure', node: form },
json && { id: 'json', label: 'JSON', node: json }, json && { id: 'json', label: 'JSON', node: json },
].filter(Boolean) as { id: string; label: string; node: ReactNode }[]; ].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 ( return (
<div className={cn('flex min-h-0 flex-1 flex-col gap-3 p-3 lg:hidden', className)}> <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"> <TabsList className="w-full">
{tabs.map((t) => ( {tabs.map((t) => (
<TabsTrigger key={t.id} value={t.id} className="flex-1"> <TabsTrigger key={t.id} value={t.id} className="flex-1">
@@ -25,12 +25,17 @@ interface MonitorLayoutProps {
* </PageShell> * </PageShell>
*/ */
export function MonitorLayout({ kpis, children, className }: MonitorLayoutProps) { 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 ( return (
<ScrollArea className={cn('flex-1', className)}> <div className={cn('flex min-h-0 flex-1 flex-col', className)}>
<div className="space-y-4 p-4 sm:p-6"> <ScrollArea className="flex-1">
{kpis} <div className="space-y-4 p-4 sm:p-6">
{children} {kpis}
</div> {children}
</ScrollArea> </div>
</ScrollArea>
</div>
); );
} }