Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .changelog/next/fixed-issue-4145.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Data Manager (/data) no longer double-scrolls and double-pads: the route is now full-width, so the page's own header bar + scrolling body is the only scroll container
4 changes: 4 additions & 0 deletions client/src/components/Layout.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,10 @@ export function SingleNavRow({ item, collapsed, active, badgeCount, pinned, onTo
const EXACT_FULL_WIDTH_PATHS = [
'/character',
'/ai',
// Data Manager is a bordered title bar over a `flex-1 overflow-auto` body,
// so it owns its own scroll. EXACT, not a prefix — a `/data` prefix would
// also swallow the `/datadog` redirect route.
'/data',
'/devtools/flows',
'/ask',
// OpenClaw lives under the Settings nav group; it's a full-bleed
Expand Down
23 changes: 23 additions & 0 deletions client/src/components/Layout.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,26 @@ describe('Layout — Game workspace scroll mode', () => {
});
});

// Data Manager renders its own bordered title bar over a `flex-1 overflow-auto`
// body, so it needs the bare full-width main. While it was missing from the
// tables it nested that scroller inside `<main>`'s own `overflow-auto p-4
// md:p-6` — two scrollbars, doubled padding (#4145).
describe('Layout — Data Manager scroll mode', () => {
it('gives /data the bare full-width main, and leaves /devtools/datadog padded', async () => {
const dataManager = await renderLayout('/data');
const dataMain = dataManager.container.querySelector('#main-content');
expect(dataMain?.className).toContain('overflow-hidden');
expect(dataMain?.className).not.toContain('overflow-auto');
expect(dataMain?.className).not.toContain('p-4');
dataManager.unmount();

const dataDog = await renderLayout('/devtools/datadog');
const dataDogMain = dataDog.container.querySelector('#main-content');
expect(dataDogMain?.className).toContain('overflow-auto');
expect(dataDogMain?.className).toContain('p-4');
});
});

describe('Layout — dynamic third-level navigation', () => {
it('collapses and expands the Series and Universes children', async () => {
api.listPipelineSeries.mockResolvedValue([{ id: 'series-1', name: 'Example Series' }]);
Expand Down Expand Up @@ -252,6 +272,9 @@ describe('Layout — isFullWidthRoute classification', () => {
// Apps: detail editor is full-width, but the Add App form is explicitly excluded
// (it has no internal scroll container and would clip below the fold).
['/apps/create', false], ['/apps/create/', false], ['/apps/a1', true], ['/apps/a1/tab', true],
// Data Manager owns its own bar+scroll shell, and is registered EXACT so
// it can't leak onto the DataDog routes that share the `/data` prefix.
['/data', true], ['/datadog', false], ['/devtools/datadog', false],
// Whole-section prefixes, and the default for an unlisted route.
['/songbook', true], ['/', false],
])('%s -> %s', (pathname, expected) => {
Expand Down
13 changes: 10 additions & 3 deletions client/src/pages/DataManager.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -444,6 +444,9 @@ export default function DataManager() {
setBackups(prev => prev.filter(b => b.name !== filename));
};

// `fullHeight` + `padded` + the `p-4` bar/body mirror the loaded shell below,
// and stay correct now that `/data` is an `isFullWidthRoute` — its `<main>`
// supplies neither the scroll container nor the padding.
if (loading) {
return (
<PageSkeleton
Expand All @@ -465,10 +468,14 @@ export default function DataManager() {

const maxSize = overview?.categories?.[0]?.size || 1;

// `/data` is an `isFullWidthRoute` (Layout.jsx), so `<main>` is a bare
// `relative overflow-hidden` and this shell owns the only scroll region.
// `min-h-0` on the column and on the body keeps a tall body scrolling instead
// of stretching the shell past `<main>` (#4145).
return (
<div className="flex flex-col h-full">
<div className="flex flex-col h-full min-h-0">
{/* Header */}
<div className="flex items-center justify-between p-4 border-b border-port-border">
<div className="shrink-0 flex items-center justify-between p-4 border-b border-port-border">
<div className="flex items-center gap-3">
<HardDrive className="w-8 h-8 text-port-accent" />
<div>
Expand All @@ -492,7 +499,7 @@ export default function DataManager() {
</div>

{/* Content */}
<div className="flex-1 overflow-auto p-4">
<div className="flex-1 min-h-0 overflow-auto p-4">
{/* Summary cards */}
<div className="grid grid-cols-2 sm:grid-cols-4 gap-3 mb-4">
<div className="bg-port-card rounded-lg p-3 border border-port-border">
Expand Down
54 changes: 54 additions & 0 deletions client/src/pages/DataManager.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -262,3 +262,57 @@ describe('DataManager busy categories (#3342)', () => {
await waitFor(() => expect(screen.getByRole('button', { name: /Purge/ })).toBeInTheDocument());
});
});

// `/data` is an `isFullWidthRoute`, so Layout's `<main>` is a bare
// `relative overflow-hidden`: this page must supply exactly ONE scroll region
// and all of its own padding. Before #4145 the route was NOT full-width, so the
// page's shell nested inside a padded, scrolling `<main>` — two scrollbars and
// doubled padding. Assert the shape so a revert on either side fails here.
describe('DataManager full-width shell (#4145)', () => {
beforeEach(() => {
getDataOverview.mockReset().mockResolvedValue(overview);
getDataCategory.mockReset().mockResolvedValue({ key: 'mystery-dir', items: [] });
});

it('renders a single h-full column whose only scroll container is the body', async () => {
const { container } = render(<DataManager />);
await waitFor(() => expect(screen.getByText(UNKNOWN_DESCRIPTION)).toBeInTheDocument());

const root = container.firstElementChild;
expect(root.className).toContain('h-full');
expect(root.className).toContain('flex-col');
// The root itself never scrolls — it fills `<main>` exactly.
expect(root.className).not.toMatch(/overflow-(auto|y-auto|scroll)/);

// Exactly one scrolling region in the page shell (the category list's own
// inner `max-h-64 overflow-auto` only exists on an expanded row).
const scrollers = [...root.children].filter((el) => /overflow-auto/.test(el.className));
expect(scrollers).toHaveLength(1);
expect(scrollers[0].className).toContain('flex-1');
expect(scrollers[0].className).toContain('min-h-0');
// The page owns its padding — Layout's full-width main supplies none.
expect(scrollers[0].className).toContain('p-4');
});

it('keeps the header bar out of the scroll region', async () => {
const { container } = render(<DataManager />);
await waitFor(() => expect(screen.getByText(UNKNOWN_DESCRIPTION)).toBeInTheDocument());

const bar = container.firstElementChild.firstElementChild;
expect(bar).toContainElement(screen.getByRole('heading', { name: 'Data Manager' }));
expect(bar.className).toContain('shrink-0');
expect(bar.className).not.toMatch(/overflow-(auto|y-auto|scroll)/);
});

it('reserves the same shell in the loading skeleton', () => {
// Never resolves — hold the page in its loading state.
getDataOverview.mockReset().mockReturnValue(new Promise(() => {}));
const { container } = render(<DataManager />);

const skeleton = container.querySelector('[aria-busy="true"]');
expect(skeleton.className).toContain('h-full');
const skeletonScrollers = [...skeleton.children].filter((el) => /overflow-y-auto/.test(el.className));
expect(skeletonScrollers).toHaveLength(1);
expect(skeletonScrollers[0].className).toContain('p-4');
});
});