tiennm99/ccs
1
0
Fork 0
mirror of https://github.com/tiennm99/ccs.git synced 2026-07-16 08:17:11 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(ui): address PR-Agent round 3 review on ConfigLayout duplicate DOM

Browse Source 
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.
This commit is contained in:
Tam Nhu Tran
2026-04-25 12:48:06 -04:00
parent caef89a7aa
commit 6261e150e3
1 changed files with 44 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files
ui/src/components/config-layout/config-layout.tsx
+44 -14
View File
@@ -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) => (