refactor: Move overflow-auto to content wrapper for proper sticky behavior
Problem:
Trying to make sticky work inside a scrollable container is complex:
- Different offsets for fullscreen vs WP-Admin
- MutationObserver to detect mode changes
- Fragile and hard to maintain
Root Cause:
<main overflow-auto> ← Scrollable container
<SubmenuBar sticky> ← Sticky inside scrollable
<SettingsLayout>
<div sticky> ← Nested sticky, complex offsets
Better Approach:
Move overflow-auto from <main> to content wrapper:
Before:
<main overflow-auto>
<SubmenuBar sticky>
<div p-4>
<AppRoutes />
After:
<main flex flex-col>
<SubmenuBar sticky> ← Sticky outside scrollable ✅
<div overflow-auto p-4> ← Only content scrolls ✅
<AppRoutes />
Benefits:
✅ Submenu always sticky (outside scroll container)
✅ Sticky header simple: just top-0
✅ No mode detection needed
✅ No MutationObserver
✅ Works everywhere: fullscreen, WP-Admin, standalone
✅ Cleaner, more maintainable code
Changes:
1. App.tsx:
- <main>: overflow-auto → flex flex-col min-h-0
- Content wrapper: p-4 → flex-1 overflow-auto p-4
2. SettingsLayout.tsx:
- Removed fullscreen detection
- Removed MutationObserver
- Simplified to: sticky top-0 (always)
Layout Structure (All Modes):
┌─────────────────────────────────────┐
│ Header / TopNav │
├─────────────────────────────────────┤
│ <main flex flex-col> │
│ ┌─────────────────────────────┐ │
│ │ SubmenuBar (sticky) │ │ ← Always sticky
│ ├─────────────────────────────┤ │
│ │ <div overflow-auto> │ │ ← Scroll here
│ │ Sticky Header (top-0) │ │ ← Simple!
│ │ Gap (mb-6) │ │
│ │ Content... │ │
│ └─────────────────────────────┘ │
└─────────────────────────────────────┘
Result:
✅ Simpler code (removed 20+ lines)
✅ More reliable behavior
✅ Easier to understand
✅ Works in all modes without special cases
Files Modified:
- App.tsx: Restructured scroll containers
- SettingsLayout.tsx: Simplified sticky logic
This commit is contained in:
@@ -414,13 +414,13 @@ function Shell() {
|
|||||||
isDesktop ? (
|
isDesktop ? (
|
||||||
<div className="flex flex-1 min-h-0">
|
<div className="flex flex-1 min-h-0">
|
||||||
<Sidebar />
|
<Sidebar />
|
||||||
<main className="flex-1 overflow-auto">
|
<main className="flex-1 flex flex-col min-h-0">
|
||||||
{isDashboardRoute ? (
|
{isDashboardRoute ? (
|
||||||
<DashboardSubmenuBar items={main.children} fullscreen={true} />
|
<DashboardSubmenuBar items={main.children} fullscreen={true} />
|
||||||
) : (
|
) : (
|
||||||
<SubmenuBar items={main.children} fullscreen={true} />
|
<SubmenuBar items={main.children} fullscreen={true} />
|
||||||
)}
|
)}
|
||||||
<div className="p-4">
|
<div className="flex-1 overflow-auto p-4">
|
||||||
<AppRoutes />
|
<AppRoutes />
|
||||||
</div>
|
</div>
|
||||||
</main>
|
</main>
|
||||||
@@ -433,8 +433,10 @@ function Shell() {
|
|||||||
) : (
|
) : (
|
||||||
<SubmenuBar items={main.children} fullscreen={true} />
|
<SubmenuBar items={main.children} fullscreen={true} />
|
||||||
)}
|
)}
|
||||||
<main className="flex-1 p-4 overflow-auto">
|
<main className="flex-1 flex flex-col min-h-0">
|
||||||
|
<div className="flex-1 overflow-auto p-4">
|
||||||
<AppRoutes />
|
<AppRoutes />
|
||||||
|
</div>
|
||||||
</main>
|
</main>
|
||||||
</div>
|
</div>
|
||||||
)
|
)
|
||||||
@@ -446,8 +448,10 @@ function Shell() {
|
|||||||
) : (
|
) : (
|
||||||
<SubmenuBar items={main.children} fullscreen={false} />
|
<SubmenuBar items={main.children} fullscreen={false} />
|
||||||
)}
|
)}
|
||||||
<main className="flex-1 p-4 overflow-auto">
|
<main className="flex-1 flex flex-col min-h-0">
|
||||||
|
<div className="flex-1 overflow-auto p-4">
|
||||||
<AppRoutes />
|
<AppRoutes />
|
||||||
|
</div>
|
||||||
</main>
|
</main>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|||||||
@@ -33,31 +33,11 @@ export function SettingsLayout({
|
|||||||
}
|
}
|
||||||
};
|
};
|
||||||
|
|
||||||
// Detect fullscreen mode
|
|
||||||
const [isFullscreen, setIsFullscreen] = React.useState(false);
|
|
||||||
|
|
||||||
React.useEffect(() => {
|
|
||||||
const checkFullscreen = () => {
|
|
||||||
setIsFullscreen(document.querySelector('.woonoow-fullscreen-root') !== null);
|
|
||||||
};
|
|
||||||
|
|
||||||
checkFullscreen();
|
|
||||||
// Re-check on route changes or mode toggles
|
|
||||||
const observer = new MutationObserver(checkFullscreen);
|
|
||||||
observer.observe(document.body, { attributes: true, attributeFilter: ['class'], subtree: true });
|
|
||||||
|
|
||||||
return () => observer.disconnect();
|
|
||||||
}, []);
|
|
||||||
|
|
||||||
// In fullscreen: submenu is inside scrollable area, so offset by submenu height (49px)
|
|
||||||
// In WP-Admin: submenu is outside scrollable area, so no offset needed (top-0)
|
|
||||||
const stickyTop = isFullscreen ? 'top-[49px]' : 'top-0';
|
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="space-y-6">
|
<div className="space-y-6">
|
||||||
{/* Sticky Header with Save Button - Edge to edge */}
|
{/* Sticky Header with Save Button - Edge to edge */}
|
||||||
{onSave && (
|
{onSave && (
|
||||||
<div className={`sticky ${stickyTop} z-10 border-b bg-background/95 backdrop-blur supports-[backdrop-filter]:bg-background/60 -mx-4 px-4 mb-6`}>
|
<div className="sticky top-0 z-10 border-b bg-background/95 backdrop-blur supports-[backdrop-filter]:bg-background/60 -mx-4 px-4 mb-6">
|
||||||
<div className="container px-0 max-w-5xl mx-auto py-3 flex items-center justify-between">
|
<div className="container px-0 max-w-5xl mx-auto py-3 flex items-center justify-between">
|
||||||
<div>
|
<div>
|
||||||
<h1 className="text-lg font-semibold">{title}</h1>
|
<h1 className="text-lg font-semibold">{title}</h1>
|
||||||
|
|||||||
Reference in New Issue
Block a user