l-lessons-efe-scroll

Project lessons learned for External File Editor scrolling / provider-root sizing. Read PROACTIVELY before planning or implementing work touching provider root wrappers, frameset leaf sizing, CodeMirr...

Lessons — EFE scroll / provider-root sizing

2026-06-12 — EFE long-document scroll regression (third attempt, fixed test-first)

What we set out to do

Make documents taller than the External File Editor frame scrollable. The bug had been "fixed" twice (#1734, #2578 problem 1 / PR #2579) and regressed both times; content beyond the visible lines was clipped as if overflow:hidden.

Approach we tried first

Attempt 1 (#1734) set the provider root to flex: 1 + minHeight: 0, guarded by a jsdom test asserting the inline style attributes. Attempt 2 (#2578) added CSS [data-efe-tab-frame-id] .cm-editor { height: 100% } with no test at all. Both patched a link in the percentage-height chain without verifying the chain in a real layout engine.

Why it went wrong (root cause)

The provider root mounts inside LeafRenderer's wrapper ([data-testid=leaf-<frameId>], packages/frameset/src/leaf-renderer.tsx), which is display: block. Flex item properties are inert under a block parent — flex: 1 silently did nothing, the root became an auto-height block, grew to content height (~6000px for 120 lines), and frame-content's overflow:hidden clipped it without a scrollbar. Every height: 100% below it (cache container, .cm-editor) then resolved against the unconstrained block, so attempt 2's CSS never bit. No test owned the invariant "a long document is scrollable" in a real layout engine — jsdom does no layout, so the #1734 style-attribute test passed forever while the behavior stayed broken.

What worked instead

height: 100% on the provider root (external-file-editor-provider.tsx) — the fill contract every working provider root already used (inbox height:"100%", search/archives h-full). And a real-browser guard, written FIRST and verified red on all surfaces: e2e/external-file-editor-long-doc-scroll.spec.ts (4 host surfaces × scroller-is-scroll-container, no document/body overflow, root inside viewport, last-line sentinel visible after scroll, wheel input moves the scroller).

Watch for next time

• If you write flex: 1 on a provider root, you're probably wrong — the LeafRenderer wrapper is display: block. Provider roots fill with width/height: 100% (the inbox/search/archives/terminal contract).

• If a "make it fill / make it scroll" fix is guarded only by a jsdom style-attribute assertion, the bug is not guarded — jsdom does no layout. Use a real-browser e2e asserting behavior (scrollHeight > clientHeight, scrollTop moves, sentinel visible by bounding rect).

• If .cm-scroller/.cm-editor height: 100% "doesn't work", walk the LIVE chain with getComputedStyle + getBoundingClientRect from the scroller upward and find the first element whose rect height exceeds its parent's — percentage chains fail silently at one unconstrained link.

• If an e2e must exercise the pin route, navigate /#/p/<slug> (HashRouter) after booting / — a plain /p/<slug> path leaves the app on the write page and the test passes against the wrong surface.

• Seed EFE file content via the mock externalFiles seed (__pendingMockSeeds) and assert the doc actually loaded (view.state.doc.lines) — otherwise mock-mode readText failure yields a short error-content tab and scroll assertions go meaninglessly green.

• CodeMirror renders scrolled-into-view lines asynchronously (requestMeasure) — poll sentinel-visibility assertions; a single post-scroll evaluate races the re-render.

Would-skip-if-redoing

Auditing CSS by source-grep and ranking suspect selectors — the live computed-style chain dump found the broken link in one run. Also skip re-asserting the old contract's style attributes in new tests; pin the behavior, mention the style only as documentation.