Load pinned plots that are in view without waiting on a scroll

[epompeii]

Problem

Pinned plots on a project's Plots page could take a long time to appear when navigating to the page, while a refresh made them show up right away. Occasionally a plot would also stay on its loading skeleton until the user scrolled or refreshed.

Root cause

Each pinned plot's data is fetched lazily, gated behind an IntersectionObserver in PinnedFrame. The observer's initial notification is not reliably delivered for elements that are already in the viewport at mount time and never subsequently move. On top of that, the skeleton placeholder and the rendered chart have different heights, so as sibling plots stream in their data the layout shifts and a plot can move into view without a scroll. On an otherwise idle page that intersection change is not always reported, so an in-view plot could stay stuck on the skeleton until a scroll or refresh forced a new rendering pass.

Fix

• Measure the plot element on mount and mark it visible immediately when it is already on or near the screen, instead of waiting on the observer's initial callback.
• For plots that start below the fold, keep observing for scroll-into-view (now with a preload margin) and additionally re-check position across the initial settling window, so a layout shift that brings a plot into view is picked up without a scroll.
• Extract the viewport check into a pure isNearViewport helper with unit tests.

Testing

• New unit tests for isNearViewport and the settling constants (npx vitest run).
• biome format and biome lint clean.
• Manually verified that previously-stuck in-view plots load without a scroll.

[github-actions]

🤖 Claude Code Review

PR: #940
Base: devel
Head: u/ep/plots-eager-load
Commit: 8a3201144b8224140c90d24fdcc6f1a7edd7037d

---

I have enough context to review. Let me verify the test assertions against the implementation logic once more and check formatting conventions are consistent.

The changes are small and focused. Here is my review.

Code Review

Overall this is a clean, well-scoped fix with good test coverage and unusually thorough explanatory comments. It addresses a real bug: pinned plots that were already in view at mount could stay stuck on the loading skeleton because the IntersectionObserver's initial callback isn't reliably delivered for elements that are in view and never move. TDD was followed (pure isNearViewport helper extracted and unit-tested). Nice work.

Correctness

• isNearViewport logic is sound. I traced all 10 test cases against rect.top < viewportHeight + margin && rect.bottom > -margin and every assertion holds, including the custom-margin fold cases and the zero-rect safe default.
• Cleanup is correct. Registering onCleanup(stopObserving) synchronously inside the onMount owner (before the async retry loop) is the right call, and clearTimeout(undefined) / observer?.disconnect() are all null-safe. The old code registered onCleanup inside the async setupObserver, which could leak if the component unmounted mid-retry, so this is a genuine improvement.
• reveal() disconnecting all observers once visible is a good latch and avoids redundant work.

Performance considerations (worth noting, not blocking)

• One ResizeObserver per pinned plot, each observing document.body. On a page with many pinned plots that are all below the fold, this creates N observers that each fire on every body layout change and call getBoundingClientRect(). Reads inside a ResizeObserver callback run post-layout so they generally don't force an extra reflow, but it's still O(N) observers reacting to shared layout churn as sibling plots stream data in. For typical pin counts this is fine; if boards can hold dozens of plots it may be worth measuring. A shared/single observer or a small debounce would reduce the fan-out if it ever shows up in profiling.
• ResizeObserver on document.body won't fire if the page uses an inner scroll container with a fixed-height body. In that layout the "sibling streams in and shifts me into view" case would fall back to the IntersectionObserver only. The comment acknowledges this is heuristic, so it's an accepted limitation rather than a defect.

Edge case (minor)

• Zero-rect "load anyway" default ({top:0, bottom:0} → near) is deliberate and reasonable. The only risk is a scenario where all offscreen plots measure as 0,0 (e.g., rendered inside an initially display:none/collapsed container), which would eagerly load every plot and defeat lazy loading. Given onMount runs after DOM insertion and each FallbackPlot skeleton has real height, this is unlikely in the current usage in Pinned.tsx, but it's the one path where the safe default could cause a request flood.

Standards compliance

• No emdashes in comments/copy. ✓
• Tabs, Biome-consistent formatting. ✓
• Pure helper extracted to util.ts and tested with Vitest, matching the console's testing conventions. ✓
• SolidJS reactivity/onMount/onCleanup used idiomatically; window/document access is confined to onMount (client-only), and the pure util.ts helper stays SSR-safe. ✓

Nits

• PLOT_PRELOAD_MARGIN is used both as the IntersectionObserver rootMargin (all four sides) and as the vertical-only margin in isNearViewport. That's consistent for vertical scrolling, but the two express "near" slightly differently (4-sided vs. vertical-only). Not a problem in practice; just a note that the single constant serves two subtly different roles.
• I couldn't execute the new tests here (vitest isn't installed in services/console/node_modules), so I verified the assertions by hand rather than by running npx vitest run. Recommend confirming the suite passes in CI.

Verdict: Looks good to merge. No correctness blockers; the only follow-up worth keeping in mind is the per-plot ResizeObserver fan-out if pinned-plot counts get large.

---

_{Model: claude-opus-4-8}