From 6261e150e34769cbba9d65e8059c5146798e7e37 Mon Sep 17 00:00:00 2001
From: Tam Nhu Tran <kaitran.ntt@gmail.com>
Date: Sat, 25 Apr 2026 12:48:06 -0400
Subject: [PATCH] fix(ui): address PR-Agent round 3 review on ConfigLayout
 duplicate DOM

Render exactly one layout at a time -- desktop 3-pane grid OR mobile
tabs -- via a useIsDesktop matchMedia hook gated at the lg breakpoint.

Previously the component rendered BOTH and toggled visibility via
Tailwind 'hidden lg:grid' / 'lg:hidden'. This duplicated FormSection
elements in the DOM, so document.getElementById() in SectionRail's
scroll-spy and click-to-jump would resolve to the (hidden) desktop copy
first on mobile -- making the rail attach to the wrong scroller and
never update the active section.
---
 .../config-layout/config-layout.tsx           | 58 ++++++++++++++-----
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/ui/src/components/config-layout/config-layout.tsx b/ui/src/components/config-layout/config-layout.tsx
index c48ccf3d..518fd877 100644
--- a/ui/src/components/config-layout/config-layout.tsx
+++ b/ui/src/components/config-layout/config-layout.tsx
@@ -1,7 +1,32 @@
-import { useState, type ReactNode } from 'react';
+import { useEffect, useState, type ReactNode } from 'react';
 import { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs';
 import { cn } from '@/lib/utils';
 
+const DESKTOP_BREAKPOINT_PX = 1024;
+
+/**
+ * Track whether the viewport meets the desktop breakpoint.
+ * Used to render EITHER the 3-pane grid OR the mobile tabs — never both.
+ * Rendering both at once duplicates FormSection ids in the DOM, which breaks
+ * SectionRail scroll-spy (getElementById returns the hidden desktop copy first).
+ */
+function useIsDesktop(): boolean {
+  const [isDesktop, setIsDesktop] = useState(() =>
+    typeof window === 'undefined'
+      ? true // SSR-safe default; harmless on first paint
+      : window.matchMedia(`(min-width: ${DESKTOP_BREAKPOINT_PX}px)`).matches
+  );
+
+  useEffect(() => {
+    const mql = window.matchMedia(`(min-width: ${DESKTOP_BREAKPOINT_PX}px)`);
+    const onChange = (e: MediaQueryListEvent) => setIsDesktop(e.matches);
+    mql.addEventListener('change', onChange);
+    return () => mql.removeEventListener('change', onChange);
+  }, []);
+
+  return isDesktop;
+}
+
 interface ConfigLayoutProps {
   /** Left rail: <ListPane> for multi-entity, <SectionRail> for single-entity, omit for none. */
   left?: ReactNode;
@@ -24,16 +49,22 @@ interface ConfigLayoutProps {
  * - <ConfigLayout …/>                          // no rail
  */
 export function ConfigLayout({ left, form, json, className }: ConfigLayoutProps) {
-  return (
-    <>
-      {/* Desktop: 3-pane grid */}
+  const isDesktop = useIsDesktop();
+
+  // CRITICAL: render exactly one layout at a time. Rendering both and
+  // toggling visibility via Tailwind `hidden lg:grid` would mount two copies
+  // of every FormSection — SectionRail's scroll-spy and click-to-jump use
+  // document.getElementById() which would resolve to the (hidden) desktop
+  // copy first on mobile, breaking the rail entirely.
+  if (isDesktop) {
+    return (
       <div
         className={cn(
-          'hidden min-h-0 flex-1 lg:grid lg:gap-4 lg:p-4',
-          left && json && 'lg:grid-cols-[260px_minmax(0,1fr)_360px]',
-          left && !json && 'lg:grid-cols-[260px_minmax(0,1fr)]',
-          !left && json && 'lg:grid-cols-[minmax(0,1fr)_360px]',
-          !left && !json && 'lg:grid-cols-1',
+          'grid min-h-0 flex-1 gap-4 p-4',
+          left && json && 'grid-cols-[260px_minmax(0,1fr)_360px]',
+          left && !json && 'grid-cols-[260px_minmax(0,1fr)]',
+          !left && json && 'grid-cols-[minmax(0,1fr)_360px]',
+          !left && !json && 'grid-cols-1',
           className
         )}
       >
@@ -45,11 +76,10 @@ export function ConfigLayout({ left, form, json, className }: ConfigLayoutProps)
           <aside className="min-w-0 overflow-hidden rounded-xl border bg-card">{json}</aside>
         )}
       </div>
+    );
+  }
 
-      {/* Mobile/tablet: tabs */}
-      <MobileTabs left={left} form={form} json={json} className={className} />
-    </>
-  );
+  return <MobileTabs left={left} form={form} json={json} className={className} />;
 }
 
 function MobileTabs({
@@ -71,7 +101,7 @@ function MobileTabs({
   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)}>
+    <div className={cn('flex min-h-0 flex-1 flex-col gap-3 p-3', className)}>
       <Tabs value={active} onValueChange={setSelected}>
         <TabsList className="w-full">
           {tabs.map((t) => (