From 8bf52e3583ab27124481f5d08015aa194595785d Mon Sep 17 00:00:00 2001 From: Adam Eivy Date: Wed, 2 Sep 2026 04:49:41 +0000 Subject: [PATCH] fix: convert 24 pages to PageSkeleton so first paint stops reflowing (#5659) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two dozen pages returned a bare sentence or a centered spinner from their top-level `if (loading)` branch, across five inconsistent idioms. Because those branches dropped the page's own header and tab strip, the entire above-the-fold region appeared at once when data arrived and shoved the viewport down — on a 360x640 phone that reflow is the whole visible screen. Each page now renders the shared `PageSkeleton` with the axes that match its loaded shape (`header`, `layout`, `tabs`, `padded`, `fullHeight`), and each call carries a specific screen-reader label rather than the bare default, so the busy region says what is loading. Pages whose header is already painted (AI Providers, Prompt Manager, Insights overview, Workspace Contexts) keep rendering it and reserve only the body with `header="none"`. Insights' local `SummaryCardSkeleton` copy is gone in favour of the shared primitive; Models' `` Suspense fallback picks up a real label. `BrailleSpinner` stays for inline busy states inside buttons and cards. Tests: a new `client/src/pages/loadingSkeletons.test.jsx` mounts all 24 pages with their fetch still in flight and asserts each announces an `aria-busy` status region with its own label, plus a tree-wide source guard that fails when any page's top-level loading branch stops rendering `PageSkeleton` or ships the default label. `PageSkeleton.test.jsx` gains the newly used prop combinations (`header="bar"` + `fullHeight`, `layout="grid"` + `header="none"`). Claude-Session: https://claude.ai/code/session_01DxNA8g7B5hZd4uswfUM1xn --- .../src/components/ui/PageSkeleton.test.jsx | 23 +++ client/src/pages/AIProviders.jsx | 5 +- client/src/pages/AgentsPage.jsx | 11 +- client/src/pages/BrainScanReport.jsx | 10 +- client/src/pages/CatalogIngredient.jsx | 12 +- client/src/pages/CreativeDirector.jsx | 16 +- client/src/pages/Game.jsx | 28 ++- client/src/pages/Insights.jsx | 24 +-- client/src/pages/Instances.jsx | 13 +- client/src/pages/Media3DDetail.jsx | 11 +- client/src/pages/Models.jsx | 2 +- client/src/pages/PipelineContinuityBible.jsx | 14 +- client/src/pages/PipelineExport.jsx | 15 +- client/src/pages/PipelineIssue.jsx | 19 +- client/src/pages/PipelineManuscriptEditor.jsx | 15 +- client/src/pages/PipelineReverseOutline.jsx | 14 +- client/src/pages/PipelineSeries.jsx | 17 +- client/src/pages/PipelineSeriesRoadmap.jsx | 16 +- client/src/pages/PipelineVoiceFingerprint.jsx | 16 +- client/src/pages/PromptManager.jsx | 6 +- client/src/pages/QuotaBurn.jsx | 15 +- client/src/pages/QuotaBurn.test.jsx | 4 +- client/src/pages/RoundEditor.jsx | 16 +- client/src/pages/RoundEditor.test.jsx | 5 +- client/src/pages/SongBookViewer.jsx | 15 +- client/src/pages/Templates.jsx | 13 +- client/src/pages/VideoTimelineEditor.jsx | 13 +- client/src/pages/WorkspaceContexts.jsx | 14 +- client/src/pages/loadingSkeletons.test.jsx | 174 ++++++++++++++++++ 29 files changed, 488 insertions(+), 68 deletions(-) create mode 100644 client/src/pages/loadingSkeletons.test.jsx diff --git a/client/src/components/ui/PageSkeleton.test.jsx b/client/src/components/ui/PageSkeleton.test.jsx index a8652c96bc..25f2e59f57 100644 --- a/client/src/components/ui/PageSkeleton.test.jsx +++ b/client/src/components/ui/PageSkeleton.test.jsx @@ -88,6 +88,29 @@ describe('PageSkeleton', () => { expect(container.innerHTML).not.toContain('px-3 py-2 sm:px-4 sm:py-3'); }); + it('owns the height on a fullHeight bar page and gives the body the scroll', () => { + // The bar branch keeps the shell `h-full` and puts `overflow-y-auto` on the + // BODY, not the root — a full-bleed page's header bar must not scroll away. + const { container } = render(); + expect(status().className).toContain('h-full'); + expect(status().className).not.toContain('overflow-y-auto'); + const bodyRegion = container.querySelector('.flex-1.min-h-0'); + expect(bodyRegion.className).toContain('overflow-y-auto'); + expect(bodyRegion.className).toContain('p-4'); + }); + + it('reserves a card grid under a page-supplied header for layout="grid" + header="none"', () => { + const { container } = render( + + ); + // No title/action placeholders — the page already painted its own chrome. + expect(cardCount(container)).toBe(3); + expect(container.innerHTML).toContain('lg:grid-cols-3'); + expect(container.innerHTML).not.toContain('lg:grid-cols-[1fr_360px]'); + // 3 cards x (1 title + 2 body lines) and nothing else. + expect(container.querySelectorAll('.animate-pulse')).toHaveLength(9); + }); + it('omits body padding on a full-bleed tab even in bar mode', () => { const { container } = render(); const bodyRegion = container.querySelector('.flex-1.min-h-0'); diff --git a/client/src/pages/AIProviders.jsx b/client/src/pages/AIProviders.jsx index ee27d8d08f..e3dc4a953e 100644 --- a/client/src/pages/AIProviders.jsx +++ b/client/src/pages/AIProviders.jsx @@ -21,6 +21,7 @@ import { } from '../utils/formatters'; import SettingsTabsHeader from '../components/settings/SettingsTabsHeader'; import PageHeader from '../components/PageHeader'; +import PageSkeleton from '../components/ui/PageSkeleton'; import OverflowMenu from '../components/ui/OverflowMenu'; import EffortSelect from '../components/cos/EffortSelect'; import Drawer from '../components/Drawer'; @@ -692,8 +693,8 @@ export default function AIProviders() {
-
-
Loading providers...
+
+
); diff --git a/client/src/pages/AgentsPage.jsx b/client/src/pages/AgentsPage.jsx index 9e9b1c9bd1..736902da84 100644 --- a/client/src/pages/AgentsPage.jsx +++ b/client/src/pages/AgentsPage.jsx @@ -3,6 +3,7 @@ import { RefreshCw, Activity, XCircle, Cpu, MemoryStick, Terminal } from 'lucide import * as api from '../services/api'; import { useAutoRefetch } from '../hooks/useAutoRefetch'; import { formatDateTime } from '../utils/formatters'; +import PageSkeleton from '../components/ui/PageSkeleton'; export function AgentsPage() { const [killing, setKilling] = useState({}); @@ -32,7 +33,15 @@ export function AgentsPage() { const totalMemory = agents.reduce((sum, a) => sum + (a.memory || 0), 0); if (loading) { - return
Scanning for AI agents...
; + return ( + + ); } return ( diff --git a/client/src/pages/BrainScanReport.jsx b/client/src/pages/BrainScanReport.jsx index 03fa3327d6..b0f34a920c 100644 --- a/client/src/pages/BrainScanReport.jsx +++ b/client/src/pages/BrainScanReport.jsx @@ -3,7 +3,7 @@ import { Link, useParams } from 'react-router'; import { ArrowLeft, FileText, RefreshCw, ShieldAlert, ShieldCheck, Skull } from 'lucide-react'; import * as api from '../services/api'; import MarkdownOutput from '../components/cos/MarkdownOutput'; -import BrailleSpinner from '../components/BrailleSpinner'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { useAutoRefetch } from '../hooks/useAutoRefetch'; const VERDICT_STYLES = { @@ -36,7 +36,13 @@ export default function BrainScanReport() { if (loading) { return ( -
+
); } diff --git a/client/src/pages/CatalogIngredient.jsx b/client/src/pages/CatalogIngredient.jsx index 533eae119f..4190bd5344 100644 --- a/client/src/pages/CatalogIngredient.jsx +++ b/client/src/pages/CatalogIngredient.jsx @@ -14,6 +14,7 @@ import Modal from '../components/ui/Modal.jsx'; import ConfirmButtonPair from '../components/ui/ConfirmButtonPair.jsx'; import UnsavedChangesConfirm from '../components/ui/UnsavedChangesConfirm.jsx'; import AutoSizeTextarea from '../components/ui/AutoSizeTextarea'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { getCatalogIngredientDetails, updateCatalogIngredient, @@ -420,9 +421,14 @@ export default function CatalogIngredient() { if (loading || !record) { return ( -
-
Loading ingredient…
-
+ ); } diff --git a/client/src/pages/CreativeDirector.jsx b/client/src/pages/CreativeDirector.jsx index b627f9ebee..d1b8fd555c 100644 --- a/client/src/pages/CreativeDirector.jsx +++ b/client/src/pages/CreativeDirector.jsx @@ -18,6 +18,7 @@ import { listUniverses } from '../services/apiUniverseBuilder.js'; import { listPipelineSeries } from '../services/apiPipeline.js'; import ModelSelect from '../components/ModelSelect'; import PageHeader from '../components/PageHeader'; +import PageSkeleton from '../components/ui/PageSkeleton'; import Drawer from '../components/Drawer'; import DirectiveComposer from '../components/creative-director/DirectiveComposer.jsx'; import CreativeDirectorModelsDrawer from '../components/creative-director/CreativeDirectorModelsDrawer.jsx'; @@ -259,7 +260,20 @@ export default function CreativeDirector() { }; if (loading) { - return
Loading projects…
; + return ( + + ); } return ( diff --git a/client/src/pages/Game.jsx b/client/src/pages/Game.jsx index 03f88f2186..a74f20cfec 100644 --- a/client/src/pages/Game.jsx +++ b/client/src/pages/Game.jsx @@ -1,6 +1,7 @@ import { useCallback, useEffect, useMemo, useState } from 'react'; import { ArrowLeft, Boxes, Gamepad2, Images, MessageSquare, Plus } from 'lucide-react'; import { Link, useNavigate, useParams } from 'react-router'; +import PageSkeleton from '../components/ui/PageSkeleton'; import toast from '../components/ui/Toast'; import AppContextPicker from '../components/AppContextPicker.jsx'; import GameBindings from '../components/games/GameBindings.jsx'; @@ -259,7 +260,32 @@ export default function Game() { }; if (loading) { - return
Loading Game studio…
; + // The detail workspace is a full-bleed h-full shell with its own bordered + // bar; the index is a plain padded page. `id` is known before the fetch + // settles, so each reserves the chrome its own loaded state renders. + return id ? ( + + ) : ( + + ); } if (id && !game) { diff --git a/client/src/pages/Insights.jsx b/client/src/pages/Insights.jsx index 32c9bfa5b2..bedb7fbd32 100644 --- a/client/src/pages/Insights.jsx +++ b/client/src/pages/Insights.jsx @@ -17,6 +17,7 @@ import GoalScorecardTab from '../components/insights/GoalScorecardTab'; import ConfidenceBadge from '../components/insights/ConfidenceBadge'; import PageHeader from '../components/PageHeader'; import TabPills from '../components/ui/TabPills'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { timeAgo } from '../utils/formatters'; // Exported for the nav-manifest tab-coverage guard (server/lib/navManifest.test.js). @@ -28,17 +29,6 @@ export const TABS = [ { id: 'goal-scorecard', label: 'Goal Scorecard', icon: Target } ]; -function SummaryCardSkeleton() { - return ( -
-
-
-
-
-
- ); -} - export function OverviewTab() { const navigate = useNavigate(); const [loading, setLoading] = useState(true); @@ -72,11 +62,13 @@ export function OverviewTab() { if (loading) { return ( -
- - - -
+ ); } diff --git a/client/src/pages/Instances.jsx b/client/src/pages/Instances.jsx index dc9a7e4635..31f7543e43 100644 --- a/client/src/pages/Instances.jsx +++ b/client/src/pages/Instances.jsx @@ -30,6 +30,7 @@ import BrainParitySchedule from '../components/instances/BrainParitySchedule'; import TailnetHelpBanner from '../components/instances/TailnetHelpBanner'; import { timeAgo, timeUntil } from '../utils/formatters'; import { directionalCounts, describeDirectional } from '../lib/syncCounts'; +import PageSkeleton from '../components/ui/PageSkeleton'; const STATUS_COLORS = { online: 'text-port-success', @@ -1295,9 +1296,15 @@ export default function Instances() { if (loading) { return ( -
-
Loading instances...
-
+ ); } diff --git a/client/src/pages/Media3DDetail.jsx b/client/src/pages/Media3DDetail.jsx index 773ca299fd..0b08c0cea2 100644 --- a/client/src/pages/Media3DDetail.jsx +++ b/client/src/pages/Media3DDetail.jsx @@ -12,6 +12,7 @@ import ImageTo3dRenderOptions from '../components/media/ImageTo3dRenderOptions'; import { fieldsFromRun, renderOptionsBody, runWantsTransparency } from '../lib/imageTo3dRenderOptions'; import { imageTo3dStatusMeta } from '../components/media/imageTo3dStatus'; import toast from '../components/ui/Toast'; +import PageSkeleton from '../components/ui/PageSkeleton'; // Poll cadence while a render is in flight (a real TRELLIS.2 render is multi-minute). const POLL_INTERVAL_MS = 2500; @@ -107,8 +108,14 @@ export default function Media3DDetail() { if (loading) { return ( -
- Loading 3D model… +
+
); } diff --git a/client/src/pages/Models.jsx b/client/src/pages/Models.jsx index 56b3db8a76..6a3ae19cd1 100644 --- a/client/src/pages/Models.jsx +++ b/client/src/pages/Models.jsx @@ -96,7 +96,7 @@ export default function Models() {
{/* Local boundary rather than the App-level one: a lazy tab must not blank out the section header and tab bar while its chunk loads. */} - }> + }> {DetailContent ? : }
diff --git a/client/src/pages/PipelineContinuityBible.jsx b/client/src/pages/PipelineContinuityBible.jsx index 350361d1d1..6dfee6980d 100644 --- a/client/src/pages/PipelineContinuityBible.jsx +++ b/client/src/pages/PipelineContinuityBible.jsx @@ -14,6 +14,7 @@ import { useEffect, useRef, useState } from 'react'; import { Link, useParams, useNavigate } from 'react-router'; import { Loader2, RefreshCw, X, ArrowLeft, BookOpen, AlertTriangle, BookMarked, Lock } from 'lucide-react'; import toast from '../components/ui/Toast'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { getPipelineSeries, getContinuityBible, @@ -112,9 +113,16 @@ export default function PipelineContinuityBible() { if (loading) { return ( -
- -
+ ); } diff --git a/client/src/pages/PipelineExport.jsx b/client/src/pages/PipelineExport.jsx index 75440ccc64..ccf7f5e5e0 100644 --- a/client/src/pages/PipelineExport.jsx +++ b/client/src/pages/PipelineExport.jsx @@ -14,6 +14,7 @@ import { useEffect, useMemo, useState } from 'react'; import { Link, useParams, useNavigate } from 'react-router'; import { Loader2, ArrowLeft, Download, Save, BookText, FileText, FileType } from 'lucide-react'; import toast from '../components/ui/Toast'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { getPipelineSeries, updatePipelineSeries, @@ -109,9 +110,17 @@ export default function PipelineExport() { if (loading) { return ( -
- -
+ ); } diff --git a/client/src/pages/PipelineIssue.jsx b/client/src/pages/PipelineIssue.jsx index d5a9ec3fa8..a3f83feb58 100644 --- a/client/src/pages/PipelineIssue.jsx +++ b/client/src/pages/PipelineIssue.jsx @@ -13,6 +13,7 @@ import { LayoutGrid, Image as ImageIcon, Clapperboard, Users, Settings, Mic, Lock, Unlock, } from 'lucide-react'; import toast from '../components/ui/Toast'; +import PageSkeleton from '../components/ui/PageSkeleton'; import Modal from '../components/ui/Modal'; import TabPills from '../components/ui/TabPills'; import { @@ -287,7 +288,23 @@ export default function PipelineIssue() { ? `${PIPELINE_STAGE_LABELS[stageId]} stage is locked — unlock it to regenerate` : ambientLockHint; - if (loading) return
Loading issue…
; + if (loading) { + return ( + + ); + } if (!issue) return null; const StageComponent = STAGE_COMPONENTS[stageId]; diff --git a/client/src/pages/PipelineManuscriptEditor.jsx b/client/src/pages/PipelineManuscriptEditor.jsx index 07ad386d07..55d3bd8531 100644 --- a/client/src/pages/PipelineManuscriptEditor.jsx +++ b/client/src/pages/PipelineManuscriptEditor.jsx @@ -34,6 +34,7 @@ import { } from 'lucide-react'; import { formatManuscript } from '../lib/manuscriptFormat'; import toast from '../components/ui/Toast'; +import PageSkeleton from '../components/ui/PageSkeleton'; import UnsavedChangesConfirm from '../components/ui/UnsavedChangesConfirm'; import { useAsyncAction } from '../hooks/useAsyncAction'; import { usePipelineProgress } from '../hooks/usePipelineProgress'; @@ -733,7 +734,19 @@ export default function PipelineManuscriptEditor() { else sectionRefs.current.delete(number); }; - if (loading) return
Loading manuscript…
; + if (loading) { + return ( + + ); + } return ( // The full-bleed route removes Layout's default scroll container. The diff --git a/client/src/pages/PipelineReverseOutline.jsx b/client/src/pages/PipelineReverseOutline.jsx index 98f53ec0ca..8234a68c85 100644 --- a/client/src/pages/PipelineReverseOutline.jsx +++ b/client/src/pages/PipelineReverseOutline.jsx @@ -12,6 +12,7 @@ import { useEffect, useRef, useState } from 'react'; import { Link, useParams, useNavigate } from 'react-router'; import { Loader2, RefreshCw, X, ArrowLeft, BookOpen, AlertTriangle, Compass } from 'lucide-react'; import toast from '../components/ui/Toast'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { getPipelineSeries, getReverseOutline, @@ -104,9 +105,16 @@ export default function PipelineReverseOutline() { if (loading) { return ( -
- -
+ ); } diff --git a/client/src/pages/PipelineSeries.jsx b/client/src/pages/PipelineSeries.jsx index d7633958ef..249087b9fe 100644 --- a/client/src/pages/PipelineSeries.jsx +++ b/client/src/pages/PipelineSeries.jsx @@ -18,6 +18,7 @@ import { Fingerprint, Plus, Trash2, Wand2, Check, X, Download, } from 'lucide-react'; import toast from '../components/ui/Toast'; +import PageSkeleton from '../components/ui/PageSkeleton'; import ArcCanvas from '../components/pipeline/ArcCanvas'; import AutopilotPanel from '../components/pipeline/AutopilotPanel'; import SeriesReviewPanel from '../components/pipeline/SeriesReviewPanel'; @@ -165,7 +166,21 @@ export default function PipelineSeries() { if (didSave) toast.success('Series saved'); }; - if (loading) return
Loading series…
; + if (loading) { + return ( + + ); + } if (!series) return null; // Mobile = flex column (grid template ignored); lg+ = grid where the inline diff --git a/client/src/pages/PipelineSeriesRoadmap.jsx b/client/src/pages/PipelineSeriesRoadmap.jsx index eeeaf5c753..32f3fba54a 100644 --- a/client/src/pages/PipelineSeriesRoadmap.jsx +++ b/client/src/pages/PipelineSeriesRoadmap.jsx @@ -22,6 +22,7 @@ import { ArcRoadmapChart } from '../components/pipeline/ArcCanvas'; import ReaderPanelView from '../components/pipeline/ReaderPanelView'; import ComparativeRankView from '../components/pipeline/ComparativeRankView'; import TabPills from '../components/ui/TabPills'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { getPipelineSeries, getIssueEditorial, analyzeIssueEditorial, getSeriesJudge, } from '../services/api'; @@ -270,9 +271,18 @@ export default function PipelineSeriesRoadmap() { if (loading) { return ( -
- Loading reader map… -
+ ); } diff --git a/client/src/pages/PipelineVoiceFingerprint.jsx b/client/src/pages/PipelineVoiceFingerprint.jsx index c46755d808..96c326dbca 100644 --- a/client/src/pages/PipelineVoiceFingerprint.jsx +++ b/client/src/pages/PipelineVoiceFingerprint.jsx @@ -18,8 +18,9 @@ import { useEffect, useState } from 'react'; import { Link, useParams, useNavigate } from 'react-router'; -import { Loader2, ArrowLeft, Fingerprint, BookOpen, Info } from 'lucide-react'; +import { ArrowLeft, Fingerprint, BookOpen, Info } from 'lucide-react'; import toast from '../components/ui/Toast'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { getPipelineSeries, getVoiceFingerprint } from '../services/api'; // A cell is an outlier when the (issue, metricKey) pair is in the drift set. @@ -59,9 +60,16 @@ export default function PipelineVoiceFingerprint() { if (loading) { return ( -
- -
+ ); } diff --git a/client/src/pages/PromptManager.jsx b/client/src/pages/PromptManager.jsx index 0ff644c384..8ec698bb24 100644 --- a/client/src/pages/PromptManager.jsx +++ b/client/src/pages/PromptManager.jsx @@ -2,7 +2,6 @@ import { useState, useEffect, useRef, useMemo } from 'react'; import { useSearchParams } from 'react-router'; import { FileText, Variable, RefreshCw, Save, Plus, Trash2, Eye, Briefcase, Search, X, ChevronRight, ChevronDown } from 'lucide-react'; import toast from '../components/ui/Toast'; -import BrailleSpinner from '../components/BrailleSpinner'; import ProviderModelSelector from '../components/ProviderModelSelector'; import { filterSelectableModels, getProviderTimeout } from '../utils/providers'; import { @@ -14,6 +13,7 @@ import { } from '../utils/formatters'; import useFieldDraft from '../hooks/useFieldDraft'; import SettingsTabsHeader from '../components/settings/SettingsTabsHeader'; +import PageSkeleton from '../components/ui/PageSkeleton'; import PageHeader from '../components/PageHeader'; import { FormField } from '../components/ui/FormField'; import Modal from '../components/ui/Modal'; @@ -462,8 +462,8 @@ export default function PromptManager() {
-
- +
+
); diff --git a/client/src/pages/QuotaBurn.jsx b/client/src/pages/QuotaBurn.jsx index c805c22727..25f98db20b 100644 --- a/client/src/pages/QuotaBurn.jsx +++ b/client/src/pages/QuotaBurn.jsx @@ -18,7 +18,7 @@ import { useNavigate, useParams } from 'react-router'; import { AlertTriangle, Flame, RefreshCw } from 'lucide-react'; import toast from '../components/ui/Toast'; import Banner from '../components/ui/Banner'; -import BrailleSpinner from '../components/BrailleSpinner'; +import PageSkeleton from '../components/ui/PageSkeleton'; import FamilyCard from '../components/quotaBurn/FamilyCard'; import { NumberField } from '../components/quotaBurn/fields'; import * as api from '../services/api'; @@ -376,7 +376,18 @@ export default function QuotaBurn() { // fades while a poll that keeps failing deserves a standing indicator. }; - if (loading) return
Loading burn plan…
; + if (loading) { + return ( + + ); + } // A failed first read used to land here with no cause and no way out but a // browser reload — the header (and its "Refresh quota") returns above. if (!config) { diff --git a/client/src/pages/QuotaBurn.test.jsx b/client/src/pages/QuotaBurn.test.jsx index e94d75be1a..961f3e10dd 100644 --- a/client/src/pages/QuotaBurn.test.jsx +++ b/client/src/pages/QuotaBurn.test.jsx @@ -490,7 +490,9 @@ describe('QuotaBurn catalog failure', () => { // and a retry can put it back — so nothing announces it without a live region. api.getQuotaBurnCatalog.mockRejectedValueOnce(new Error('Catalog request failed')); renderPage('/devtools/quota-burn/grok'); - const banner = await screen.findByRole('status'); + // Scope to the banner's own text: the first-paint PageSkeleton is also a + // `status` region, so a bare role query would race it. + const banner = (await screen.findByText('Job choices could not be loaded')).closest('[role="status"]'); expect(banner).toHaveAttribute('aria-live', 'polite'); expect(banner).toHaveTextContent('Job choices could not be loaded'); }); diff --git a/client/src/pages/RoundEditor.jsx b/client/src/pages/RoundEditor.jsx index fd3fa3ada7..4f6655fa3d 100644 --- a/client/src/pages/RoundEditor.jsx +++ b/client/src/pages/RoundEditor.jsx @@ -32,6 +32,7 @@ import ReferenceAnalysis from '../components/songs/ReferenceAnalysis'; import RoundEditForm from '../components/songs/RoundEditForm'; import RoundReadView from '../components/songs/RoundReadView'; import { TEMP_ID_RE } from '../lib/roundDraft.js'; +import PageSkeleton from '../components/ui/PageSkeleton'; export default function RoundEditor() { const { id } = useParams(); @@ -45,7 +46,20 @@ export default function RoundEditor() { const { partnerSongs, otherSongs, togglePartner } = useRoundPartners({ id, song, setSong }); if (loading) { - return
Loading round…
; + return ( + + ); } if (!song) { return ( diff --git a/client/src/pages/RoundEditor.test.jsx b/client/src/pages/RoundEditor.test.jsx index 72f507f8f2..5a3ef77f61 100644 --- a/client/src/pages/RoundEditor.test.jsx +++ b/client/src/pages/RoundEditor.test.jsx @@ -55,8 +55,9 @@ describe('RoundEditor round→round navigation', () => { fireEvent.click(screen.getByText('go-b')); // The old draft must be gone (so a Save can't write Song A into Song B) and - // the loading state shown until the new song arrives. - await waitFor(() => expect(screen.getByText(/Loading round/)).toBeTruthy()); + // the loading state shown until the new song arrives. That state is the + // shared PageSkeleton, which announces itself by aria-label, not by text. + await waitFor(() => expect(screen.getByRole('status', { name: 'Loading round' })).toBeTruthy()); expect(screen.queryByText('Song A')).toBeNull(); expect(screen.queryByRole('button', { name: /^Save$/ })).toBeNull(); diff --git a/client/src/pages/SongBookViewer.jsx b/client/src/pages/SongBookViewer.jsx index 34769eae8b..0ccf07b208 100644 --- a/client/src/pages/SongBookViewer.jsx +++ b/client/src/pages/SongBookViewer.jsx @@ -84,6 +84,7 @@ import { safeReadStorage, safeWriteStorage } from '../lib/safeStorage.js'; import { formatBytes, formatDurationSec } from '../utils/formatters'; import { isHttpUrl } from '../utils/urlNormalize'; import { readFileAsBase64, JSON_UPLOAD_MAX_FILE_SIZE } from '../utils/fileUpload'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { getSong, updateSong, deleteSong, listSongAttachments, uploadSongAttachment, deleteSongAttachment, songAttachmentUrl, @@ -593,7 +594,19 @@ export default function SongBookViewer() { } if (loading || !song) { - return

Loading song…

; + return ( + + ); } const stageClass = SONG_STAGE_COLORS[song.stage] || SONG_STAGE_COLORS.new; diff --git a/client/src/pages/Templates.jsx b/client/src/pages/Templates.jsx index aff4e75187..fd775bf2d9 100644 --- a/client/src/pages/Templates.jsx +++ b/client/src/pages/Templates.jsx @@ -4,6 +4,7 @@ import { Layers, Code, Server, Globe, Smartphone, MonitorSmartphone, Plus } from import * as api from '../services/api'; import FolderPicker from '../components/FolderPicker'; import { FormField } from '../components/ui/FormField'; +import PageSkeleton from '../components/ui/PageSkeleton'; const ICONS = { layers: Layers, @@ -68,9 +69,15 @@ export default function Templates() { if (loading) { return ( -
-
Loading templates...
-
+ ); } diff --git a/client/src/pages/VideoTimelineEditor.jsx b/client/src/pages/VideoTimelineEditor.jsx index 103d89be19..c0dc6b7818 100644 --- a/client/src/pages/VideoTimelineEditor.jsx +++ b/client/src/pages/VideoTimelineEditor.jsx @@ -24,6 +24,7 @@ import { TimelineBlock, FloatingLane, LibraryTile, StillTile, AudioRow, BedAudio, } from '../components/media/VideoTimelineLanes'; import { NumberField, FadeFields, RemoveButton } from '../components/media/VideoTimelineInspector'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { assetUrl, segmentDuration, @@ -647,7 +648,17 @@ export default function VideoTimelineEditor() { const overlayLabel = useCallback((ov) => ov.assetFile, []); const bedLabel = useCallback((tr) => tr.assetFile, []); - if (loading) return
Loading project…
; + if (loading) { + return ( + + ); + } if (error || !project) { return (
diff --git a/client/src/pages/WorkspaceContexts.jsx b/client/src/pages/WorkspaceContexts.jsx index 86cc859fc7..e3cf47d871 100644 --- a/client/src/pages/WorkspaceContexts.jsx +++ b/client/src/pages/WorkspaceContexts.jsx @@ -4,10 +4,10 @@ import { Layers, GitBranch, SquareTerminal, ListChecks, Save, RotateCcw, Trash2, FolderGit2, AlertCircle, CheckCircle2, ArrowRight } from 'lucide-react'; -import BrailleSpinner from '../components/BrailleSpinner'; import toast from '../components/ui/Toast'; import { useAsyncAction } from '../hooks/useAsyncAction'; import { timeAgo } from '../utils/formatters'; +import PageSkeleton from '../components/ui/PageSkeleton'; import { listWorkspaceContexts, getWorkspaceContext, saveWorkspaceContext, restoreWorkspaceContext, deleteWorkspaceContext @@ -68,7 +68,7 @@ function ContextDetail({ appId }) { }, { errorMessage: 'Failed to clear context' }); if (loading) { - return
; + return ; } if (!ctx) return null; @@ -209,7 +209,15 @@ function ContextList() { useEffect(() => { load(); }, [load]); if (loading) { - return
; + return ( + + ); } if (rows.length === 0) { diff --git a/client/src/pages/loadingSkeletons.test.jsx b/client/src/pages/loadingSkeletons.test.jsx new file mode 100644 index 0000000000..ee8c76bdf9 --- /dev/null +++ b/client/src/pages/loadingSkeletons.test.jsx @@ -0,0 +1,174 @@ +import { readFileSync, readdirSync } from 'node:fs'; +import { dirname, join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { act, render, screen } from '@testing-library/react'; +import { createMemoryRouter, RouterProvider } from 'react-router'; + +// A hoisted declaration, so the hoisted `vi.mock` factories below can reach it. +// Every exported FUNCTION resolves to a call that never settles, so the page +// stays in its first-paint state for the whole assertion; exported constants +// pass through untouched (a page that reads `PIPELINE_STAGES.includes(...)` +// during render would otherwise crash before it could paint anything). +function pendingModule(path) { + return async () => { + const actual = await vi.importActual(path); + return Object.fromEntries(Object.entries(actual).map(([name, value]) => [ + name, + typeof value === 'function' ? vi.fn(() => new Promise(() => {})) : value, + ])); + }; +} + +vi.mock('../services/api', pendingModule('../services/api')); +vi.mock('../services/apiCatalog.js', pendingModule('../services/apiCatalog.js')); +vi.mock('../services/apiCreativeDirector.js', pendingModule('../services/apiCreativeDirector.js')); +vi.mock('../services/apiImageVideo.js', pendingModule('../services/apiImageVideo.js')); +vi.mock('../services/apiPipeline.js', pendingModule('../services/apiPipeline.js')); +vi.mock('../services/apiPrompts', pendingModule('../services/apiPrompts')); +vi.mock('../services/apiProviders', pendingModule('../services/apiProviders')); +vi.mock('../services/apiUniverseBuilder.js', pendingModule('../services/apiUniverseBuilder.js')); +vi.mock('../services/socket', () => ({ + default: { on: vi.fn(), off: vi.fn(), emit: vi.fn(), connected: false }, +})); + +const PAGES_DIR = dirname(fileURLToPath(import.meta.url)); + +// --------------------------------------------------------------------------- +// Structural guard — tree-wide, and the one that catches the actual regression +// this file exists for: a page's top-level `if (loading)` reverting to a bare +// `
Loading…
`, which exposes no `status` role and reserves no layout, +// so the header/tabs pop in and shove the viewport down (#2843, #5659). +// --------------------------------------------------------------------------- +const LOADING_GUARD = /^ {2}if \((?:loading|isLoading)\b[^)]*\)\s*(\{)?/; + +// Reading the source (rather than rendering all ~50 pages) is deliberate: it +// covers every page including ones whose render harness would need a bespoke +// mock, and it keeps working as pages are added. +function loadingGuardBodies(source) { + const lines = source.split('\n'); + const bodies = []; + lines.forEach((line, i) => { + const match = LOADING_GUARD.exec(line); + if (!match) return; + if (match[1]) { + // Braced block: consume until the brace depth returns to zero. + let depth = 0; + const block = []; + for (let j = i; j < lines.length; j += 1) { + block.push(lines[j]); + depth += (lines[j].match(/\{/g) || []).length - (lines[j].match(/\}/g) || []).length; + if (j > i && depth <= 0) break; + } + bodies.push({ line: i + 1, body: block.join('\n') }); + return; + } + // Single-expression guard: consume until the statement terminates. + const block = [lines[i]]; + for (let j = i + 1; !block[block.length - 1].trimEnd().endsWith(';') && j < lines.length; j += 1) { + block.push(lines[j]); + } + bodies.push({ line: i + 1, body: block.join('\n') }); + }); + return bodies; +} + +const pageFiles = readdirSync(PAGES_DIR) + .filter((f) => f.endsWith('.jsx') && !f.includes('.test.')) + .sort(); + +describe('page loading states reserve their layout', () => { + it('renders PageSkeleton from every top-level loading guard', () => { + const offenders = []; + pageFiles.forEach((file) => { + const source = readFileSync(join(PAGES_DIR, file), 'utf8'); + loadingGuardBodies(source).forEach(({ line, body }) => { + // `return null` / bare `return` render nothing at all, so there is no + // chrome to reflow — they are not skeleton candidates. + if (/return\s*(null)?\s*;/.test(body) && !body.includes('<')) return; + if (!body.includes('PageSkeleton')) offenders.push(`${file}:${line}`); + }); + }); + // A bare `
Loading…
` exposes no `status` role and reserves no + // layout, so the header/tabs pop in and shove the viewport down. + expect(offenders).toEqual([]); + }); + + it('gives every PageSkeleton a label naming what is loading, never the bare default', () => { + const offenders = []; + pageFiles.forEach((file) => { + const source = readFileSync(join(PAGES_DIR, file), 'utf8'); + // One entry per `` element, self-closing or not. + const calls = source.match(//g) || []; + calls.forEach((call) => { + const label = /label="([^"]*)"/.exec(call); + if (!label || label[1].trim() === '' || label[1] === 'Loading') offenders.push(`${file}: ${call.split('\n')[0]}`); + }); + }); + expect(offenders).toEqual([]); + }); +}); + +// --------------------------------------------------------------------------- +// Render guard — the pages converted in #5659, each mounted with its fetch +// still in flight. A bare text loader exposes no `status` role, so these fail +// loudly if one comes back. +// --------------------------------------------------------------------------- +const RENDERED = [ + ['AgentsPage', () => import('./AgentsPage').then((m) => m.AgentsPage), '/devtools/agents', '/devtools/agents', 'Scanning for AI agents'], + ['AIProviders', () => import('./AIProviders'), '/ai', '/ai', 'Loading providers'], + ['BrainScanReport', () => import('./BrainScanReport'), '/brain/links/:id/scan-report', '/brain/links/l1/scan-report', 'Loading scan report'], + ['CatalogIngredient', () => import('./CatalogIngredient'), '/catalog/:type/:id', '/catalog/idea/i1', 'Loading ingredient'], + ['CreativeDirector', () => import('./CreativeDirector'), '/creative-director', '/creative-director', 'Loading Creative Director projects'], + ['Game', () => import('./Game'), '/game', '/game', 'Loading Game studio'], + ['Insights (OverviewTab)', () => import('./Insights').then((m) => m.OverviewTab), '/insights/overview', '/insights/overview', 'Loading insights overview'], + ['Instances', () => import('./Instances'), '/instances', '/instances', 'Loading instances'], + ['Media3DDetail', () => import('./Media3DDetail'), '/3d/:id', '/3d/abc', 'Loading 3D model'], + ['PipelineContinuityBible', () => import('./PipelineContinuityBible'), '/pipeline/series/:seriesId/continuity', '/pipeline/series/s1/continuity', 'Loading series continuity'], + ['PipelineExport', () => import('./PipelineExport'), '/pipeline/series/:seriesId/export', '/pipeline/series/s1/export', 'Loading export options'], + ['PipelineIssue', () => import('./PipelineIssue'), '/pipeline/issues/:issueId/:stageId', '/pipeline/issues/i1/outline', 'Loading pipeline issue'], + ['PipelineManuscriptEditor', () => import('./PipelineManuscriptEditor'), '/pipeline/series/:seriesId/manuscript', '/pipeline/series/s1/manuscript', 'Loading manuscript'], + ['PipelineReverseOutline', () => import('./PipelineReverseOutline'), '/pipeline/series/:seriesId/outline', '/pipeline/series/s1/outline', 'Loading reverse outline'], + ['PipelineSeries', () => import('./PipelineSeries'), '/pipeline/series/:seriesId', '/pipeline/series/s1', 'Loading series'], + ['PipelineSeriesRoadmap', () => import('./PipelineSeriesRoadmap'), '/pipeline/series/:seriesId/roadmap', '/pipeline/series/s1/roadmap', 'Loading reader map'], + ['PipelineVoiceFingerprint', () => import('./PipelineVoiceFingerprint'), '/pipeline/series/:seriesId/voice', '/pipeline/series/s1/voice', 'Loading voice fingerprint'], + ['PromptManager', () => import('./PromptManager'), '/prompts', '/prompts', 'Loading prompts'], + ['QuotaBurn', () => import('./QuotaBurn'), '/devtools/quota-burn', '/devtools/quota-burn', 'Loading burn plan'], + ['RoundEditor', () => import('./RoundEditor'), '/rounds/:id', '/rounds/r1', 'Loading round'], + ['SongBookViewer', () => import('./SongBookViewer'), '/songbook/:id', '/songbook/s1', 'Loading song'], + ['Templates', () => import('./Templates'), '/templates', '/templates', 'Loading app templates'], + ['VideoTimelineEditor', () => import('./VideoTimelineEditor'), '/media/timeline/:projectId', '/media/timeline/p1', 'Loading timeline project'], + ['WorkspaceContexts', () => import('./WorkspaceContexts'), '/workspace-contexts', '/workspace-contexts', 'Loading workspace projects'], +]; + +describe('converted pages announce a labelled busy region on first paint', () => { + beforeEach(() => { + // jsdom ships no `matchMedia`, and a page that reads one on mount throws + // before it can paint its skeleton. + window.matchMedia = vi.fn((query) => ({ + matches: false, + media: query, + addEventListener: vi.fn(), + removeEventListener: vi.fn(), + })); + }); + + // A DATA router, not ``: pages that guard unsaved edits call + // `useBlocker`, which throws outside one. + it.each(RENDERED)('%s', async (_name, load, routePath, entry, label) => { + const mod = await load(); + const Page = mod.default || mod; + const router = createMemoryRouter( + [{ path: routePath, element: }], + { initialEntries: [entry] }, + ); + render(); + // Settle the mount effects that fire before the (never-settling) fetch, so + // the assertion isn't racing an act() warning. + await act(async () => {}); + + const status = screen.getAllByRole('status')[0]; + expect(status).toHaveAttribute('aria-busy', 'true'); + expect(status).toHaveAttribute('aria-label', label); + }); +});