Skip to content

fix(frontend): use skeletons for loading and add a slow-response note - #1333

Merged
IAmJSD merged 4 commits into
LeanerCloud:mainfrom
IAmJSD:ui/issue-186
Oct 24, 2026
Merged

IAmJSD merged 4 commits into
LeanerCloud:mainfrom
IAmJSD:ui/issue-186

Conversation

@IAmJSD

@IAmJSD IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator

Loading states were inconsistent: most panels used shimmer skeletons, but several showed plain "Loading..." text (sometimes next to a skeleton), a slow response looked like a hang, and the Opportunities skeleton was invisible because its cells collapsed to 0px. This PR moves the remaining block-level text loaders to skeletons, fixes the Opportunities skeleton, and adds an app-wide "still loading" note after 8 s.

Refs #186
Partial: text loaders that live inside <option> or <label> elements (where a shimmer cannot render) are left as text, and there is no request timeout or abort, only a visible note.

What changed

  • frontend/src/lib/skeleton.ts: every show* helper now adds a screen-reader-only "Loading…" status (the shimmer stays aria-hidden) and arms an 8 s timer (SKELETON_SLOW_MS). If the skeleton is still on screen and its panel is not hidden when the timer fires, it appends "Still loading. This is taking longer than usual." with role=status. A success render that replaces the children, or teardownSkeleton, turns the timer into a no-op. New showSkeletonText(container, lines) helper.
  • Text loaders moved to skeletons:
    • riexchange.ts: loadAutomationSettings (4 lines) and loadExchangeHistory (3x7 rows).
    • settings.ts: loadGlobalSettings (#settings-loading) and loadOverridesPanel.
    • apikeys.ts: the create-key permissions list.
  • styles/base.css: .skeleton-table is full width with fixed layout and a minimum box width; .skeleton-slow-note is centered, muted and spans grid rows (Home KPI tiles, Plans cards).
  • styles/responsive.css: #recommendations-list > table.skeleton-table keeps display: table, so the Opportunities skeleton cells no longer collapse to 0px.
  • Commits: be1d55d2 (the change), 8aa8baa9 (tests).

Why this approach

Skeletons were already the dominant pattern (#344), so the remaining text loaders were moved to them rather than the reverse. The slow fallback lives in the shared helper, so all of the roughly 15 skeleton call sites get it without per-module code. It checks whether shimmer elements are still present rather than a data marker, because most success renders replace content via innerHTML and never call teardown. It only adds a note and never aborts the request, so a slow but valid response still lands. Empty responses are already handled by each renderer's own empty state; the remaining "looks like a hang" case was a skeleton left on screen, which the note covers.

Screenshots

Before is main (a14dc8f2), after is this branch. Both are headless Chromium at 1440x900 against the repo's mocked e2e API. The API routes in these scenarios never resolve.

Before After
slow-opps before slow-opps after
ri-exchange-loading before ri-exchange-loading after
settings-loading before settings-loading after
  • Opportunities, slow load (9 s): before, the skeleton rows have 0-width shimmer, so the panel is a blank card. After, the shimmer spans all columns and the "Still loading. This is taking longer than usual." note appears after 8 s.
  • RI Exchange history: before, plain "Loading exchange history..." text next to skeleton tables. After, the same skeleton rows as the rest of the page.
  • Admin > General: before, "Loading settings..." text. After, a text skeleton in place of the form.

Verification

Run on this branch's head 8aa8baa9 on top of main a14dc8f2, from frontend/:

  • Lint: 0 errors (142 warnings, all already on main). Typecheck and build pass.
  • Jest (--maxWorkers=2): 109 suites, 3497 passed, 1 skipped, 0 failed. The changed suites also pass with --randomize (32 tests).
  • New jest tests: skeleton.test.ts +10 with fake timers (showSkeletonText; one screen-reader status per skeleton kind; note appears exactly at the threshold; no note after an innerHTML handoff, after teardown, or in a hidden panel; re-show restarts the clock without stacking notes). riexchange-automation-settings.test.ts +1 (skeleton instead of "Loading settings...").
  • Playwright (e2e-local.sh): 158 passed, 0 failed, including the new tests-e2e/loading-fallback.spec.ts, which holds the recommendations request open and checks the skeleton has non-zero width, the note appears after 8 s of page clock, and both clear when data arrives.
  • Secrets scan clean (planted key flagged); scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.

Notes for review

  • This is a subset of the issue. Left as text on purpose: the RI Exchange target-offering <option> placeholder, federation.ts and ladder-account-selector.ts select/label text (a shimmer cannot sit inside an <option> or label), the ladder.ts initial template "Loading…", the ladder-timeline status text, and the Archera comparison live-region text (screen-reader only).
  • Open for the UI owner: the 8 s threshold and the note copy; the extra sr-only "Loading…" status per skeleton.
  • Skeleton tables now use fixed layout (equal-width columns), which slightly changes the look of the RI Exchange and History skeletons.
  • The note does not cancel or retry the request. A real timeout or abort would need AbortController plumbing in api/client.ts, which is left out of this PR.

Overlaps

Note for the UI owner

This changes UI code that @cristim owns. IAmJSD reviewed the before/after screenshots and asked for it to be merged; any design point you disagree with can be a follow-up.

🤖 Generated with Claude Code

IAmJSD and others added 2 commits October 24, 2026 00:36
…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 LeanerCloud#186

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d 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 LeanerCloud#186

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD IAmJSD added priority/p2 Backlog-worthy type/chore Maintenance / non-user-visible triaged Item has been triaged severity/low Minor harm urgency/this-quarter Within the quarter impact/few Limited audience effort/s Hours labels Oct 24, 2026
@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Caution

Review failed

The pull request is closed.

Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

IAmJSD and others added 2 commits October 24, 2026 16:32
Conflicts:
- frontend/src/__tests__/riexchange-automation-settings.test.ts: both
  sides appended a describe block at the end of the file (this branch's
  LeanerCloud#186 skeleton loading-state test, main's LeanerCloud#37 value-attribute escaping
  tests from LeanerCloud#1318). Kept both blocks, LeanerCloud#186 first.
- frontend/src/settings.ts: both sides added an import after the ladder
  import (showSkeletonText here, buildLastUpdatedStamp from LeanerCloud#1314 on
  main). Kept both imports.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts:
- frontend/src/lib/skeleton.ts: kept both accessibility layers. Main's
  aria-busy + MutationObserver (LeanerCloud#729/LeanerCloud#1370) and this branch's sr-only
  "Loading…" status and 8 s "Still loading" note both run from
  activateSkeleton; teardownSkeleton clears the slow timer and aria-busy.
- frontend/src/__tests__/skeleton.test.ts: kept both suites (slow fallback
  and aria-busy); the aria-busy table also covers showSkeletonText.
- frontend/src/riexchange.ts: kept main's titled "Exchange Automation"
  frame (LeanerCloud#190) with aria-busy and put this branch's showSkeletonText inside
  it, replacing the visible "Loading exchange automation settings..." line.
  Imports unioned (skeletonText no longer used).
- frontend/src/__tests__/riexchange-automation-settings.test.ts: kept main's
  LeanerCloud#190 frame tests; folded this branch's assertions (skeleton, sr-only
  status, no loading paragraph) into the loading test.
- frontend/src/apikeys.ts, frontend/src/settings.ts: import unions.
- frontend/src/styles/responsive.css: kept this branch's skeleton-table
  display:table rule and main's scroll-affordance rules (LeanerCloud#185).

Also updated tests-e2e/purchasing-sections.spec.ts (from main) to expect
the skeleton inside the frame instead of the removed loading paragraph.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): APPROVE e6add163490a30d72b034092f21d55ffd84b3e8d

Blocking:

@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Local verification at e6add163 merged onto current main (f2ab6731; merge-tree aed59292; Linux, Node, headless Chromium):

  • This branch's own checks passed at this head: lint, typecheck, build, the full jest suite, the full Playwright e2e suite, and the git-secrets checks (see Verification in the PR body).
  • The combined tree of every UI PR in this merge wave (tree 04af8e9b, main plus 5 PRs) passes npm ci, lint, typecheck, build, jest (1 skipped, 4122 passed, 4123 total), and the full Playwright suite (238 passed).
  • CI is green on e6add163 (CI Success, Run pre-commit hooks).
  • The independent review above approves this exact head.
  • Nothing was applied to any cloud.

@IAmJSD
IAmJSD marked this pull request as ready for review October 24, 2026 20:51
@IAmJSD
IAmJSD merged commit c5158e3 into LeanerCloud:main Oct 24, 2026
29 checks passed
@IAmJSD

IAmJSD commented Oct 24, 2026

Copy link
Copy Markdown
Collaborator Author

Marked ready and merged by IAmJSD's agent, at IAmJSD's request (IAmJSD reviewed the before/after screenshots; cristim is traveling), after the independent review and local verification above, all at e6add163.

IAmJSD added a commit to IAmJSD/cloud-commitments-platform that referenced this pull request Oct 24, 2026
Conflicts:
- frontend/src/riexchange.ts: import block. Main (LeanerCloud#1333) replaced the
  skeletonText import with showSkeletonText; this branch added the
  statusBadgeHtml import. Kept main's skeleton import and this branch's
  status-badge import.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/s Hours impact/few Limited audience priority/p2 Backlog-worthy severity/low Minor harm triaged Item has been triaged type/chore Maintenance / non-user-visible urgency/this-quarter Within the quarter

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant