mirror of
https://github.com/tiennm99/ccs.git
synced 2026-10-12 20:15:40 +00:00
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.
This commit is contained in:
1 parent
caef89a7aa
commit
6261e150e3
1 file changed
+44
-14
@@ -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 { Tabs, TabsContent, TabsList, TabsTrigger } from '@/components/ui/tabs';
|
||||||
import { cn } from '@/lib/utils';
|
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 {
|
interface ConfigLayoutProps {
|
||||||
/** Left rail: <ListPane> for multi-entity, <SectionRail> for single-entity, omit for none. */
|
/** Left rail: <ListPane> for multi-entity, <SectionRail> for single-entity, omit for none. */
|
||||||
left?: ReactNode;
|
left?: ReactNode;
|
||||||
@@ -24,16 +49,22 @@ interface ConfigLayoutProps {
|
|||||||
* - <ConfigLayout …/> // no rail
|
* - <ConfigLayout …/> // no rail
|
||||||
*/
|
*/
|
||||||
export function ConfigLayout({ left, form, json, className }: ConfigLayoutProps) {
|
export function ConfigLayout({ left, form, json, className }: ConfigLayoutProps) {
|
||||||
return (
|
const isDesktop = useIsDesktop();
|
||||||
<>
|
|
||||||
{/* Desktop: 3-pane grid */}
|
// 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
|
<div
|
||||||
className={cn(
|
className={cn(
|
||||||
'hidden min-h-0 flex-1 lg:grid lg:gap-4 lg:p-4',
|
'grid min-h-0 flex-1 gap-4 p-4',
|
||||||
left && json && 'lg:grid-cols-[260px_minmax(0,1fr)_360px]',
|
left && json && 'grid-cols-[260px_minmax(0,1fr)_360px]',
|
||||||
left && !json && 'lg:grid-cols-[260px_minmax(0,1fr)]',
|
left && !json && 'grid-cols-[260px_minmax(0,1fr)]',
|
||||||
!left && json && 'lg:grid-cols-[minmax(0,1fr)_360px]',
|
!left && json && 'grid-cols-[minmax(0,1fr)_360px]',
|
||||||
!left && !json && 'lg:grid-cols-1',
|
!left && !json && 'grid-cols-1',
|
||||||
className
|
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>
|
<aside className="min-w-0 overflow-hidden rounded-xl border bg-card">{json}</aside>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
{/* Mobile/tablet: tabs */}
|
return <MobileTabs left={left} form={form} json={json} className={className} />;
|
||||||
<MobileTabs left={left} form={form} json={json} className={className} />
|
|
||||||
</>
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function MobileTabs({
|
function MobileTabs({
|
||||||
@@ -71,7 +101,7 @@ function MobileTabs({
|
|||||||
const active = tabs.some((t) => t.id === selected) ? selected : (tabs[0]?.id ?? 'form');
|
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', className)}>
|
||||||
<Tabs value={active} onValueChange={setSelected}>
|
<Tabs value={active} onValueChange={setSelected}>
|
||||||
<TabsList className="w-full">
|
<TabsList className="w-full">
|
||||||
{tabs.map((t) => (
|
{tabs.map((t) => (
|
||||||
|
|||||||
Reference in new issue
Block a user