From 5d0c01a6e8e04a5a537d6e8c09e78b2e77560b6a Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Mon, 26 Oct 2026 09:14:50 +0000 Subject: [PATCH] fix(frontend): use the loading skeleton for ladder configs and the static settings placeholder Issue #186 made skeletons the one loading pattern, with a "Still loading" note after 8s, but two plain-text loaders were left: the per-account ladder config table showed a "Loading..." paragraph while its configs loaded, and index.html's #settings-loading said "Loading settings..." until loadGlobalSettings replaced it. The ladder table now shows skeleton rows in its own 8-column shape (cleared on error too), and the static placeholder is the same skeleton markup with a screen-reader status, sized by two small CSS classes since static HTML cannot set inline sizes under the CSP. Co-Authored-By: Claude Opus 5.5 --- frontend/src/__tests__/ladder.test.ts | 35 +++++++++++++++++++++++++ frontend/src/__tests__/skeleton.test.ts | 17 ++++++++++++ frontend/src/index.html | 14 +++++++++- frontend/src/ladder-config-table.ts | 3 +++ frontend/src/ladder.ts | 12 ++++++--- frontend/src/styles/base.css | 11 ++++++++ 6 files changed, 87 insertions(+), 5 deletions(-) diff --git a/frontend/src/__tests__/ladder.test.ts b/frontend/src/__tests__/ladder.test.ts index 254cc86b9..b93ecfaf9 100644 --- a/frontend/src/__tests__/ladder.test.ts +++ b/frontend/src/__tests__/ladder.test.ts @@ -79,6 +79,41 @@ describe('ladder.ts', () => { ]); }); + // Issue #186: the per-account table loads behind the shared skeleton, not a + // plain "Loading..." paragraph, and the skeleton gives way to the table or + // to the error. + test('per-account configs show skeleton rows while loading, then the table (issue #186)', async () => { + document.body.innerHTML = '
'; + let resolve: (c: LadderConfig[]) => void = () => undefined; + (api.getLadderConfigs as jest.Mock).mockReturnValue(new Promise((r) => { resolve = r; })); + const done = initLadderingSettings(false); + + const container = document.getElementById('ladder-configs-table-container')!; + // No visible "Loading…" paragraph; the only text is the sr-only status. + expect(container.querySelector('p')).toBeNull(); + const rows = container.querySelectorAll('tr.skeleton-row'); + expect(rows).toHaveLength(3); + expect(rows[0]!.querySelectorAll('td')).toHaveLength(8); + expect(container.querySelector('.skeleton-status')).toHaveTextContent('Loading…'); + + resolve([baseConfig({ id: 'config-1' })]); + await done; + expect(container.querySelector('.skeleton')).toBeNull(); + expect(container.querySelectorAll('tbody tr')).toHaveLength(1); + }); + + test('a failed per-account config load replaces the skeleton with the error (issue #186)', async () => { + document.body.innerHTML = '
'; + (api.getLadderConfigs as jest.Mock).mockRejectedValue(new Error('boom')); + jest.spyOn(console, 'error').mockImplementation(() => undefined); + await initLadderingSettings(false); + + const container = document.getElementById('ladder-configs-table-container')!; + expect(container.querySelector('.skeleton')).toBeNull(); + expect(container.hasAttribute('aria-busy')).toBe(false); + expect(container).toHaveTextContent('Failed to load per-account configurations.'); + }); + test('config modal moves focus inside, traps Tab, closes on Escape and restores focus (#731)', async () => { await renderSection([baseConfig({ id: 'config-1' })]); const editBtn = document.querySelector('.ladder-edit-btn')!; diff --git a/frontend/src/__tests__/skeleton.test.ts b/frontend/src/__tests__/skeleton.test.ts index c4fedf9f7..d6e287078 100644 --- a/frontend/src/__tests__/skeleton.test.ts +++ b/frontend/src/__tests__/skeleton.test.ts @@ -5,6 +5,8 @@ * the DOM shape each call site expects (tile / row / box). */ +import * as fs from 'fs'; +import * as path from 'path'; import { skeletonBox, skeletonText, @@ -254,6 +256,21 @@ describe('consistent loading pattern + slow fallback (issue #186)', () => { }); }); +// Issue #186: the static Settings placeholder in index.html (shown before +// loadGlobalSettings runs) uses the same skeleton, not "Loading settings..." +// text. +test('index.html #settings-loading is a skeleton with a screen-reader status (issue #186)', () => { + const html = fs.readFileSync(path.join(__dirname, '..', 'index.html'), 'utf-8'); + const doc = new DOMParser().parseFromString(html, 'text/html'); + const loading = doc.getElementById('settings-loading')!; + expect(loading.querySelectorAll('.skeleton-group .skeleton.skeleton-line')).toHaveLength(6); + expect(loading.querySelector('.skeleton-group')!.getAttribute('aria-hidden')).toBe('true'); + const status = loading.querySelector('.skeleton-status.sr-only[role="status"]'); + expect(status?.textContent).toBe('Loading…'); + // Nothing but the screen-reader status carries text. + expect(loading.textContent!.trim()).toBe('Loading…'); +}); + // Issue #729 (UX-34): the placeholders are aria-hidden, so the container // carries aria-busy while one is showing, and drops it once real content // (or nothing) replaces it, whichever path did the replacing. diff --git a/frontend/src/index.html b/frontend/src/index.html index 7e4baac72..62009b53e 100644 --- a/frontend/src/index.html +++ b/frontend/src/index.html @@ -386,7 +386,19 @@

Admin

Global Configuration

Configure CUDly settings for commitment purchases across all cloud providers.

-
Loading settings...
+ +
+ Loading… + +