From be1d55d26f952806726f694aef44b70d6ecf616a Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 00:36:24 +0000 Subject: [PATCH 1/2] fix(frontend): standardize loading on skeletons, add a slow-response fallback The app mixed shimmer skeletons with plain "Loading..." paragraphs, sometimes side by side (RI Exchange showed skeleton tables next to "Loading exchange history..."), and a slow or never-replaced skeleton gave no sign that anything was still happening. On Opportunities the skeleton was invisible altogether: the list table is display:block for horizontal scrolling, which collapsed the auto-sized skeleton cells to their padding and the 80%-wide shimmer boxes to 0px, so the panel read as an empty white card. - lib/skeleton.ts: every show* call adds a screen-reader-only "Loading..." status (the shimmer is aria-hidden) and arms a timer. If the skeleton is still on screen after 8 s (SKELETON_SLOW_MS) and its panel is not hidden, a visible "Still loading. This is taking longer than usual." note is appended. Success renders that replace the children, or teardownSkeleton, make the timer a no-op. New showSkeletonText helper. - Text loaders moved to skeletons: RI Exchange automation settings and exchange history, Admin > General settings (#settings-loading), account service-override panels, and the API key permissions list. - CSS: skeleton tables use fixed layout at full width with a minimum box width, the Opportunities skeleton table keeps display:table, and the slow note spans grid rows (Home KPI tiles, Plans cards). Refs #186 Co-Authored-By: Claude Opus 5.5 --- frontend/src/apikeys.ts | 5 ++- frontend/src/lib/skeleton.ts | 64 ++++++++++++++++++++++++++++++ frontend/src/riexchange.ts | 17 +++----- frontend/src/settings.ts | 9 ++++- frontend/src/styles/base.css | 26 ++++++++++++ frontend/src/styles/responsive.css | 6 +++ 6 files changed, 112 insertions(+), 15 deletions(-) diff --git a/frontend/src/apikeys.ts b/frontend/src/apikeys.ts index 31cd71265..35377553b 100644 --- a/frontend/src/apikeys.ts +++ b/frontend/src/apikeys.ts @@ -9,7 +9,7 @@ import { escapeHtml, formatDateTime, formatRelativeTime } from './utils'; import { confirmDialog } from './confirmDialog'; import { showToast } from './toast'; import { openModal, closeModal } from './modal'; -import { showSkeletonRows, teardownSkeleton } from './lib/skeleton'; +import { showSkeletonRows, showSkeletonText, teardownSkeleton } from './lib/skeleton'; // State for modal management let currentApiKeys: APIKeyInfo[] = []; @@ -273,7 +273,8 @@ export async function showCreateKeyModal(): Promise { } const permissionsContainer = document.getElementById('apikey-permissions-list'); - if (permissionsContainer) permissionsContainer.innerHTML = '

Loading your permissions…

'; + // Issue #186: skeleton instead of a "Loading your permissions…" line. + if (permissionsContainer) showSkeletonText(permissionsContainer, 3); // Open the modal before the permissions fetch resolves so the dialog // never sits closed while waiting on the network. diff --git a/frontend/src/lib/skeleton.ts b/frontend/src/lib/skeleton.ts index 4229e9e8b..85177b09a 100644 --- a/frontend/src/lib/skeleton.ts +++ b/frontend/src/lib/skeleton.ts @@ -16,7 +16,44 @@ * * All helpers use `createElement` only — no `innerHTML` — to match the * codebase's XSS posture from issue #340. + * + * Issue #186: skeletons are the one loading pattern for panels (plain + * "Loading..." paragraphs were migrated to showSkeletonText). Every show* + * call also adds a screen-reader-only "Loading…" status, since the shimmer + * blocks are aria-hidden, and arms a slow-response fallback: if the + * skeleton is still on screen after SKELETON_SLOW_MS, a visible "Still + * loading" note is appended so a slow or stuck response does not look + * like a hang. + */ + +/** How long a skeleton may show before the "Still loading" note appears. */ +export const SKELETON_SLOW_MS = 8_000; + +const SLOW_NOTE_TEXT = 'Still loading. This is taking longer than usual.'; + +const slowTimers = new WeakMap>(); + +function clearSlowTimer(container: HTMLElement): void { + const t = slowTimers.get(container); + if (t !== undefined) { + clearTimeout(t); + slowTimers.delete(container); + } +} + +/** + * True while the container still shows skeleton placeholders. Success + * renders replace the children (often via innerHTML, which leaves the + * data marker behind), so the shimmer elements themselves are the signal. */ +function skeletonStillShown(container: HTMLElement): boolean { + return container.isConnected + && container.dataset['skeletonActive'] === '1' + && container.querySelector('.skeleton') !== null + // A placeholder whose panel was hidden (e.g. #settings-loading once the + // form shows) is not something the user is waiting on. + && container.closest('.hidden') === null; +} /** * A single shimmer block. Width/height are CSS sizes (e.g. "60%", "1rem", @@ -95,6 +132,32 @@ export function skeletonRow(cols: number): HTMLTableRowElement { function activateSkeleton(container: HTMLElement): void { while (container.firstChild) container.removeChild(container.firstChild); container.dataset['skeletonActive'] = '1'; + + const status = document.createElement('span'); + status.className = 'sr-only skeleton-status'; + status.setAttribute('role', 'status'); + status.textContent = 'Loading…'; + container.appendChild(status); + + clearSlowTimer(container); + slowTimers.set(container, setTimeout(() => { + slowTimers.delete(container); + if (!skeletonStillShown(container) || container.querySelector('.skeleton-slow-note')) return; + const note = document.createElement('p'); + note.className = 'skeleton-slow-note'; + note.setAttribute('role', 'status'); + note.textContent = SLOW_NOTE_TEXT; + container.appendChild(note); + }, SKELETON_SLOW_MS)); +} + +/** + * Replace the children of `container` with `lines` text-line skeletons — + * the panel/form counterpart of showSkeletonRows (issue #186). + */ +export function showSkeletonText(container: HTMLElement, lines: number): void { + activateSkeleton(container); + container.appendChild(skeletonText(lines)); } /** @@ -147,6 +210,7 @@ export function showSkeletonBlock(container: HTMLElement, width: string, height: * content. */ export function teardownSkeleton(container: HTMLElement): void { + clearSlowTimer(container); if (container.dataset['skeletonActive']) { while (container.firstChild) container.removeChild(container.firstChild); delete container.dataset['skeletonActive']; diff --git a/frontend/src/riexchange.ts b/frontend/src/riexchange.ts index 4b0855f7b..0b0addf8e 100644 --- a/frontend/src/riexchange.ts +++ b/frontend/src/riexchange.ts @@ -27,7 +27,7 @@ import type { Provider, } from './api'; import { openModal, closeModal } from './modal'; -import { showSkeletonRows, teardownSkeleton } from './lib/skeleton'; +import { showSkeletonRows, showSkeletonText, teardownSkeleton } from './lib/skeleton'; import { canAccess } from './permissions'; import { applyReadOnlySettings, isPermissionDeniedError } from './settings'; import { showToast } from './toast'; @@ -2065,11 +2065,9 @@ export async function loadAutomationSettings(): Promise { const container = document.getElementById('ri-exchange-automation-settings'); if (!container) return; - container.textContent = ''; - const loadingP = document.createElement('p'); - loadingP.className = 'loading'; - loadingP.textContent = 'Loading settings...'; - container.appendChild(loadingP); + // Issue #186: same skeleton pattern as the rest of the app (was a + // "Loading settings..." paragraph next to shimmering tables). + showSkeletonText(container, 4); try { const config = await api.getRIExchangeConfig(); @@ -2326,11 +2324,8 @@ async function loadExchangeHistory(generation: number = scopeGeneration): Promis const container = document.getElementById('ri-exchange-history-list'); if (!container) return; - container.textContent = ''; - const loadingP = document.createElement('p'); - loadingP.className = 'loading'; - loadingP.textContent = 'Loading exchange history...'; - container.appendChild(loadingP); + // Issue #186: skeleton rows shaped like the 7-column history table. + showSkeletonRows(container, 3, 7); try { const records = await api.getRIExchangeHistory(); diff --git a/frontend/src/settings.ts b/frontend/src/settings.ts index efbed45af..fa6abcb48 100644 --- a/frontend/src/settings.ts +++ b/frontend/src/settings.ts @@ -20,6 +20,7 @@ import { isServiceFilterError, } from './serviceFilters'; import { initLadderingSettings } from './ladder'; +import { showSkeletonText } from './lib/skeleton'; /** * Issue #196 — after any mutation to an `account_service_overrides` row @@ -1174,7 +1175,7 @@ function closeAccountOverridesModal(): void { } export async function loadOverridesPanel(accountId: string, panel: HTMLElement, provider: AccountProvider): Promise { - panel.textContent = 'Loading\u2026'; + showSkeletonText(panel, 2); // issue #186: was a plain "Loading…" line try { const overrides = await api.listAccountServiceOverrides(accountId); panel.textContent = ''; @@ -3167,7 +3168,11 @@ export async function loadGlobalSettings(): Promise { const formEl = document.getElementById('global-settings-form'); const errorEl = document.getElementById('settings-error'); - if (loadingEl) loadingEl.classList.remove('hidden'); + if (loadingEl) { + loadingEl.classList.remove('hidden'); + // Issue #186: skeleton instead of the static "Loading settings..." text. + showSkeletonText(loadingEl, 6); + } if (formEl) formEl.classList.add('hidden'); if (errorEl) errorEl.classList.add('hidden'); diff --git a/frontend/src/styles/base.css b/frontend/src/styles/base.css index 74c206fbd..1f2aad217 100644 --- a/frontend/src/styles/base.css +++ b/frontend/src/styles/base.css @@ -276,6 +276,32 @@ body { margin-bottom: 0; } +/* Skeleton tables (showSkeletonRows). The shimmer boxes are 80% of an + * auto-sized cell; with nothing else in the cell, auto layout shrank the + * cells to their padding and the boxes to 0px, so on Opportunities the + * "skeleton" was a blank card that read as a hang (issue #186). Fixed + * layout spreads the columns over the full width and the min-width keeps + * every box visible on narrow viewports. */ +.skeleton-table { + width: 100%; + table-layout: fixed; +} + +.skeleton-row .skeleton { + min-width: 1.5rem; +} + +/* Slow-response fallback appended by lib/skeleton.ts after + * SKELETON_SLOW_MS (issue #186). Spans the full row when the skeleton + * container is a grid (Home KPI tiles, Plans cards). */ +.skeleton-slow-note { + grid-column: 1 / -1; + margin: var(--cudly-sp-3) 0 0; + color: var(--cudly-text-muted); + font-size: var(--cudly-fs-sm); + text-align: center; +} + @keyframes cudly-shimmer { 0% { background-position: 200% 0; } 100% { background-position: -200% 0; } diff --git a/frontend/src/styles/responsive.css b/frontend/src/styles/responsive.css index 0fbe0ee62..88df077e9 100644 --- a/frontend/src/styles/responsive.css +++ b/frontend/src/styles/responsive.css @@ -362,3 +362,9 @@ body.sidebar-open { max-width: 100%; overflow-x: auto; } + +/* The loading skeleton has nothing to scroll; as a block its cells + * collapsed and the shimmer was invisible (issue #186). */ +#recommendations-list > table.skeleton-table { + display: table; +} From 8aa8baa9812b1bb693fb1e53a0b5fbfbc65229b6 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Sat, 24 Oct 2026 00:36:24 +0000 Subject: [PATCH 2/2] test(frontend): cover the skeleton status, slow-load note and migrated loaders Jest (fake timers): showSkeletonText shape, one screen-reader status per skeleton kind, the note appears exactly at SKELETON_SLOW_MS, never after an innerHTML handoff, teardown or inside a hidden panel, and re-showing restarts the clock without stacking notes. RI Exchange automation settings render the skeleton instead of "Loading settings...". Playwright: on a held recommendations request the Opportunities skeleton is actually visible (non-zero shimmer width), the note appears after 8 s of page clock, and both go away once the data arrives. Refs #186 Co-Authored-By: Claude Opus 5.5 --- .../riexchange-automation-settings.test.ts | 15 ++++ frontend/src/__tests__/skeleton.test.ts | 80 +++++++++++++++++++ frontend/tests-e2e/loading-fallback.spec.ts | 36 +++++++++ 3 files changed, 131 insertions(+) create mode 100644 frontend/tests-e2e/loading-fallback.spec.ts diff --git a/frontend/src/__tests__/riexchange-automation-settings.test.ts b/frontend/src/__tests__/riexchange-automation-settings.test.ts index 514368266..86423cc43 100644 --- a/frontend/src/__tests__/riexchange-automation-settings.test.ts +++ b/frontend/src/__tests__/riexchange-automation-settings.test.ts @@ -147,3 +147,18 @@ describe('loadAutomationSettings: graceful degradation on permission errors (iss }, ); }); + +describe('loadAutomationSettings loading state (issue #186)', () => { + test('shows the shared skeleton, not a "Loading settings..." paragraph', async () => { + setupContainer(); + let resolve: (v: unknown) => void = () => undefined; + (api.getRIExchangeConfig as jest.Mock).mockReturnValueOnce(new Promise((r) => { resolve = r; })); + const pending = loadAutomationSettings(); + const container = document.getElementById('ri-exchange-automation-settings')!; + expect(container.querySelector('.skeleton')).not.toBeNull(); + expect(container.querySelector('p.loading')).toBeNull(); + expect(container.textContent).not.toContain('Loading settings...'); + resolve(null); + await pending.catch(() => undefined); + }); +}); diff --git a/frontend/src/__tests__/skeleton.test.ts b/frontend/src/__tests__/skeleton.test.ts index feee3784c..c5cce5392 100644 --- a/frontend/src/__tests__/skeleton.test.ts +++ b/frontend/src/__tests__/skeleton.test.ts @@ -13,8 +13,10 @@ import { showSkeletonTiles, showSkeletonRows, showSkeletonBlock, + showSkeletonText, teardownSkeleton, isSkeletonActive, + SKELETON_SLOW_MS, } from '../lib/skeleton'; describe('skeleton primitives', () => { @@ -173,3 +175,81 @@ describe('skeleton show / teardown lifecycle', () => { expect(container.querySelectorAll('.skeleton')).toHaveLength(1); }); }); + +describe('consistent loading pattern + slow fallback (issue #186)', () => { + let container: HTMLElement; + beforeEach(() => { + jest.useFakeTimers(); + container = document.createElement('div'); + document.body.appendChild(container); + }); + afterEach(() => { + jest.useRealTimers(); + container.remove(); + }); + + const slowNote = (): HTMLElement | null => container.querySelector('.skeleton-slow-note'); + + test('showSkeletonText renders N shimmer lines and marks active', () => { + showSkeletonText(container, 4); + expect(container.querySelectorAll('.skeleton-group .skeleton')).toHaveLength(4); + expect(isSkeletonActive(container)).toBe(true); + }); + + test.each([ + ['tiles', (c: HTMLElement) => showSkeletonTiles(c, 2)], + ['rows', (c: HTMLElement) => showSkeletonRows(c, 2, 2)], + ['block', (c: HTMLElement) => showSkeletonBlock(c, '1px', '1px')], + ['text', (c: HTMLElement) => showSkeletonText(c, 2)], + ])('%s skeleton carries one screen-reader "Loading…" status', (_name, show) => { + show(container); + show(container); + const status = container.querySelectorAll('.skeleton-status'); + expect(status).toHaveLength(1); + expect(status[0]!.getAttribute('role')).toBe('status'); + expect(status[0]!.classList.contains('sr-only')).toBe(true); + expect(status[0]!.textContent).toBe('Loading…'); + }); + + test('a skeleton still showing after the threshold gets a "Still loading" note', () => { + showSkeletonRows(container, 3, 3); + jest.advanceTimersByTime(SKELETON_SLOW_MS - 1); + expect(slowNote()).toBeNull(); + jest.advanceTimersByTime(1); + expect(slowNote()!.textContent).toBe('Still loading. This is taking longer than usual.'); + expect(slowNote()!.getAttribute('role')).toBe('status'); + // The shimmer stays; the note is added, not swapped in. + expect(container.querySelectorAll('tr.skeleton-row')).toHaveLength(3); + }); + + test('no note when the real content replaced the skeleton (innerHTML handoff)', () => { + showSkeletonRows(container, 3, 3); + container.innerHTML = '
real
'; + jest.advanceTimersByTime(SKELETON_SLOW_MS); + expect(slowNote()).toBeNull(); + }); + + test('no note after teardownSkeleton (error path)', () => { + showSkeletonText(container, 2); + teardownSkeleton(container); + jest.advanceTimersByTime(SKELETON_SLOW_MS); + expect(slowNote()).toBeNull(); + }); + + test('no note for a skeleton inside a hidden panel', () => { + container.classList.add('hidden'); + showSkeletonText(container, 2); + jest.advanceTimersByTime(SKELETON_SLOW_MS); + expect(slowNote()).toBeNull(); + }); + + test('re-showing restarts the clock and never stacks notes', () => { + showSkeletonTiles(container, 2); + jest.advanceTimersByTime(SKELETON_SLOW_MS - 1000); + showSkeletonTiles(container, 2); + jest.advanceTimersByTime(SKELETON_SLOW_MS - 1000); + expect(slowNote()).toBeNull(); + jest.advanceTimersByTime(1000); + expect(container.querySelectorAll('.skeleton-slow-note')).toHaveLength(1); + }); +}); diff --git a/frontend/tests-e2e/loading-fallback.spec.ts b/frontend/tests-e2e/loading-fallback.spec.ts new file mode 100644 index 000000000..6d5082faf --- /dev/null +++ b/frontend/tests-e2e/loading-fallback.spec.ts @@ -0,0 +1,36 @@ +import { test, expect } from '@playwright/test'; +import { mockApi, seedAuth, RECS, SUMMARY } from './fixtures/recs'; + +// Issue #186: the Opportunities skeleton is visible, a skeleton that sits for +// a long time gets a "Still loading" note so a slow response does not look +// like a hang, and the note goes away with the skeleton once data arrives. + +test('slow recommendations load shows a still-loading note, then the table', async ({ page }) => { + await seedAuth(page); + await mockApi(page); + let release: () => void = () => undefined; + const gate = new Promise((resolve) => { release = resolve; }); + await page.route(/\/api\/recommendations(?:\?.*)?$/, async (route) => { + await gate; + await route.fulfill({ json: { recommendations: RECS, summary: SUMMARY, regions: [] } }); + }); + await page.clock.install(); + await page.goto('/opportunities'); + + const list = page.locator('#recommendations-list'); + await expect(list.locator('.skeleton-row')).toHaveCount(5); + // The shimmer must actually be visible: the Opportunities table is + // display:block for scrolling, which used to collapse skeleton cells to 0px. + const box = await list.locator('.skeleton-row .skeleton').first().boundingBox(); + expect(box!.width).toBeGreaterThan(16); + await expect(list.locator('.skeleton-status')).toHaveText('Loading…'); + await expect(list.locator('.skeleton-slow-note')).toHaveCount(0); + + await page.clock.fastForward(8_000); + await expect(list.locator('.skeleton-slow-note')).toHaveText('Still loading. This is taking longer than usual.'); + + release(); + await expect(list.locator('tr.recommendation-row').first()).toBeVisible(); + await expect(list.locator('.skeleton-slow-note')).toHaveCount(0); + await expect(list.locator('.skeleton-status')).toHaveCount(0); +});