fix: stop Data Manager double-scrolling and double-padding inside Layout's main (#4145) - #4240
Merged
Conversation
…out's main (#4145) /data was in neither EXACT_FULL_WIDTH_PATHS nor FULL_WIDTH_PATH_PREFIXES, so Layout rendered it in the default 'overflow-auto p-4 md:p-6' main while the page itself is a 'flex flex-col h-full' shell with its own bordered header bar over a 'flex-1 overflow-auto p-4' body — two nested scroll containers and two layers of padding. Registers /data in EXACT_FULL_WIDTH_PATHS (exact, not a prefix, so it can't swallow the /datadog routes) and keeps the page's shell as the single scroll container, which is what the full-width branch expects: its main is a bare 'relative overflow-hidden' that supplies neither scroll nor padding.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/data(Data Manager) appeared in neitherEXACT_FULL_WIDTH_PATHSnorFULL_WIDTH_PATH_PREFIXES, soLayoutgave it the defaultoverflow-auto p-4 md:p-6<main>. The page itself is aflex flex-col h-fullshell with its own bordered header bar over aflex-1 overflow-auto p-4body — so the page's scroller nested inside<main>'s scroller, and both applied padding.Chose option 1 from the issue (register the route as full-width) over option 2 (flatten the page into a normal scrolling page), because a sticky header bar over a scrolling body is exactly the shape the full-width branch exists for — the same shell as
/tribe,/rapid-reader,/timeline, and/songbook, all of which are registered full-width.One correction to the issue's phrasing of option 1: it suggested also dropping the page's
h-full/overflow-autowrapper "since<main>would now handle the scroll container". It would not. The full-width branch renders<main>as a barerelative overflow-hidden, andclient/src/CLAUDE.mdis explicit that a full-width page must own an internaloverflow-y-autoor it clips below the fold. So the page's shell is kept as the single scroll container; what's removed is<main>'s competing one.Registered as an exact path rather than a prefix: a
/dataprefix would also match the/datadogredirect route.Changes:
client/src/components/Layout.jsx— add/datatoEXACT_FULL_WIDTH_PATHS.client/src/pages/DataManager.jsx—min-h-0on the flex column and on the body,shrink-0on the header bar, so a tall body scrolls rather than stretching the shell past<main>. This also makes the live shell byte-for-byte equivalent to whatPageSkeletonreserves.client/src/pages/DataManager.jsx— thePageSkeletoncall'sfullHeight/padded/barClassName="p-4"/bodyClassName="p-4"props are the correct set for the new shell and are unchanged; a comment now records why (its root isflex flex-col min-h-0 h-full, barshrink-0 … p-4, bodyflex-1 min-h-0 overflow-y-auto p-4— matching the loaded page exactly). Dropping them here would have reintroduced the mismatch in the other direction, since the full-width<main>supplies neither scroll nor padding.Test plan
client/src/components/Layout.test.jsx— newLayout — Data Manager scroll modeintegration case asserting/datarenders theoverflow-hiddenmain with nooverflow-auto/p-4, while/devtools/datadogstays padded+scrolling; plus['/data', true], ['/datadog', false], ['/devtools/datadog', false]rows in theisFullWidthRouteclassification table.client/src/pages/DataManager.test.jsx— newDataManager full-width shell (#4145)suite asserting the loaded page is a non-scrollingh-fullcolumn with exactly one direct-child scroll region (flex-1 min-h-0 overflow-auto p-4), that the header bar isshrink-0and outside it, and that the loading skeleton reserves the same shape./dataentry fails both new Layout assertions (2 failures), confirming the tests are not vacuous.cd client && npm test— 640 files / 7766 tests passing.cd client && npx biome lint --error-on-warningson the four touched files — clean.Closes #4145