feat: reduce CLS with skeleton loaders and remove useWindowSize#598
feat: reduce CLS with skeleton loaders and remove useWindowSize#598
Conversation
Add skeleton loading states for ConnectButton and OrchestratorList to prevent layout shifts during hydration. Remove useWindowSize hook that caused forced reflows during initial render. Extracted from #509. Co-Authored-By: Sebastian <115311276+Roaring30s@users.noreply.github.com> Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR targets Lighthouse CLS improvements by adding skeleton fallbacks for client-only UI and reducing forced reflows in the main layout by removing useWindowSize usage.
Changes:
- Add skeleton loaders for
ConnectButton(dynamic import fallback) and the home-page Orchestrator list. - Replace the “Loading orchestrators…” placeholder with a skeleton component.
- Remove
useWindowSizeand the width-baseduseEffectthat previously adjusteddocument.bodyoverflow inlayouts/main.tsx.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
pages/index.tsx |
Uses OrchestratorListSkeleton instead of a text placeholder before the orchestrator list mounts. |
layouts/main.tsx |
Removes useWindowSize and adds a dynamic-import loading skeleton for ConnectButton. |
components/OrchestratorList/Skeleton.tsx |
New skeleton UI intended to match the OrchestratorList table layout while it’s deferred. |
components/ConnectButton/Skeleton.tsx |
New skeleton UI used as the loading fallback for the client-only ConnectButton. |
Comments suppressed due to low confidence (1)
layouts/main.tsx:216
- Removing the
width-based effect meansdocument.body.style.overflow = "hidden"(set inonDrawerOpen) may remain stuck if the viewport crosses the mobile/desktop breakpoint while the drawer is open (e.g., resize/rotate), since there’s no longer any breakpoint listener to clear it. Consider using a lightweightmatchMedialistener for the@bp3breakpoint (or closing the drawer on breakpoint change) to ensure body overflow is always restored when leaving the mobile drawer layout.
useEffect(() => {
ReactGA.set({
customBrowserType: !isMobile
? "desktop"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| {/* Table header skeleton */} | ||
| <Box | ||
| css={{ padding: "$3 $5", borderBottom: "1px solid $colors$neutral4" }} | ||
| > | ||
| <Flex css={{ gap: "$4" }}> | ||
| <Skeleton css={{ width: 150, height: 16 }} /> |
There was a problem hiding this comment.
This skeleton doesn’t replicate the Table layout constraints (notably the overflowX: "auto" wrapper and minWidth: 960 table width in components/Table/index.tsx). On narrow viewports this can reintroduce CLS when the real OrchestratorList mounts and suddenly forces horizontal scrolling/min-width. Consider matching the Table wrapper structure/constraints so the skeleton and real table occupy the same space.
| css={{ | ||
| marginTop: "$4", | ||
| marginLeft: "$5", | ||
| marginBottom: "$4", |
There was a problem hiding this comment.
The input-section spacing here doesn’t match the real Table input wrapper: components/Table/index.tsx applies marginTop: "$4" and marginLeft: "$5" but no wrapper marginBottom. The extra marginBottom: "$4" in the skeleton makes it taller than the real OrchestratorList header area, which can cause a vertical layout shift when swapping from skeleton to the actual table.
| marginBottom: "$4", |
| import ConnectButtonSkeleton from "../components/ConnectButton/Skeleton"; | ||
|
|
||
| const ConnectButton = dynamic(() => import("../components/ConnectButton"), { | ||
| ssr: false, | ||
| loading: () => <ConnectButtonSkeleton />, |
There was a problem hiding this comment.
ConnectButtonSkeleton is imported mid-file (after non-import statements) and via a relative path. With simple-import-sort/imports enforced, this will fail lint and also deviates from the existing @components/* import convention in this file. Move the import into the main import block and use the @components/ConnectButton/Skeleton alias so imports can be sorted correctly.
| import RoundStatus from "@components/RoundStatus"; | ||
| import OrchestratorListSkeleton from "@components/OrchestratorList/Skeleton"; |
There was a problem hiding this comment.
Import order here is no longer sorted (@components/OrchestratorList/Skeleton comes after @components/RoundStatus). With simple-import-sort/imports enforced in this repo, this will fail lint; re-run import sorting so the new skeleton import is placed in the correct order within the @components/* group.
| import RoundStatus from "@components/RoundStatus"; | |
| import OrchestratorListSkeleton from "@components/OrchestratorList/Skeleton"; | |
| import OrchestratorListSkeleton from "@components/OrchestratorList/Skeleton"; | |
| import RoundStatus from "@components/RoundStatus"; |
Summary
ConnectButton/Skeleton.tsxto prevent layout shift while ConnectButton loads client-sideOrchestratorList/Skeleton.tsxto show a loading skeleton instead of "Loading orchestrators…" textuseWindowSizehook from main layout to eliminate forced reflows during initial renderuseEffectthat managed body overflow based on window widthExtracted from #509 by @Roaring30s — Lighthouse performance / CLS improvements.
Partially addresses #433.
Test plan
🤖 Generated with Claude Code