[IAmJSD]

Closes #186

Why

#186 asked for one consistent loading pattern, plus a fallback so a slow or empty response does not look like a hang. Most of that is already on main. lib/skeleton.ts is the one panel loader: it adds a screen-reader "Loading…" status, and after 8s it shows a visible "Still loading. This is taking longer than usual." note (tests-e2e/loading-fallback.spec.ts). Two plain-text loaders were still left:

• The Commitment Laddering per-account table rendered a <p>Loading…</p> while its configs loaded (ladder.ts).
• index.html's static #settings-loading said "Loading settings..." until loadGlobalSettings swapped in the skeleton.

What

• ladder.ts: the per-account table container now shows showSkeletonRows(container, 3, LADDER_CONFIG_TABLE_COLUMNS). Its 8 columns match the real table, and the count is exported from ladder-config-table.ts next to the header. The error path calls teardownSkeleton before rendering the error, so no slow-note timer or aria-busy is left behind.
• index.html: #settings-loading is now the same skeleton markup that showSkeletonText produces: an sr-only role="status" "Loading…" and six aria-hidden shimmer lines. Static HTML cannot size blocks inline under the CSP, so two classes in base.css (.skeleton-line, .skeleton-line-short) give the lines the same 1rem height and 60% last line that the JS helper uses.
• Option and label loaders stay as text, as before.

Out of scope: request timeout or abort (AbortController). The issue's ask is a visible fallback for a slow or empty response, and the 8s "Still loading" note already provides it. Cancelling slow requests would be a separate change to the API client. This PR uses Closes on that reading; the owner may want a follow-up issue if aborting requests is also wanted.

Verification

• Fail-before: with main's ladder.ts and index.html, the new -t "issue #186" tests in ladder.test.ts and skeleton.test.ts give 2 failed (the ladder skeleton-rows test and the index.html placeholder test). With this change, all pass.
• npm run lint: 0 errors (141 warnings, all already on main). npm run typecheck: clean. npm run build: ok.
• npx jest --no-coverage: 171 suites, 4504 passed, 1 skipped.
• Full Playwright suite (local, private port): 267 passed.
• Secrets scan: clean, planted key flagged. scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL.
• No Go changes.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 6 billable files and costs up to $1.50.}

Or wait 58 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: LeanerCloud/cloud-commitments-platform
• Branch: main
• File: .coderabbit.yaml
• Commit: 2a11540
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 6955b21c-5363-4a56-aabb-fda5f5e1878b

Commits

Reviewing files that changed from the base of the PR and between 2a11540 and 5d0c01a.

Files selected for processing: 6

• frontend/src/__tests__/ladder.test.ts
• frontend/src/__tests__/skeleton.test.ts
• frontend/src/index.html
• frontend/src/ladder-config-table.ts
• frontend/src/ladder.ts
• frontend/src/styles/base.css

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[IAmJSD]

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

Blocking:

• none

Non-blocking:

• frontend/src/ladder-timeline.ts:147: the timeline in the same Commitment Laddering section still shows a plain 'Loading complete retained timeline...' line while it loads. Panel-level text loaders like this are what chore(frontend): standardize loading indicators and add slow/empty fallbacks #186 asks to standardize, so either skeleton it too or name it as left out. The other plain "Loading" strings left (ladder-account-selector.ts, riexchange.ts:1176 offerings, federation.ts, history-paging.ts button) are option, label or button text, which the body explicitly keeps.
• ladder.ts success path: renderConfigTable replaces the skeleton via innerHTML without teardownSkeleton, so data-skeleton-active="1" stays on #ladder-configs-table-container. It is harmless today: the MutationObserver clears aria-busy, and the slow-note timer's skeletonStillShown check finds no .skeleton. But isSkeletonActive() would report true for that container. A teardownSkeleton(container) before rendering would match the error path.

Attacked:

• Read lib/skeleton.ts to check the new call sites can't leak the 8s slow note or aria-busy. Error path: explicit teardown. Success path: covered by the observer and the timer guard, as above. Static #settings-loading: loadGlobalSettings (settings.ts:3068) immediately re-runs showSkeletonText(loadingEl, 6), so the static markup and the static aria-busy are only the pre-script paint. The .skeleton-line/-short classes reproduce skeletonText's 1rem and 60% sizes without inline styles (CSP). 8 columns matches the table header.
• Fail-before: I reverted ladder.ts, index.html and ladder-config-table.ts. Exactly the 2 new chore(frontend): standardize loading indicators and add slow/empty fallbacks #186 loading tests failed (the error-path test passes on main too, as expected). Restored, ladder.test.ts, skeleton.test.ts and settings* (12 suites, 375 tests) pass.
• Ran tsc --noEmit (clean), eslint on the changed files (0 errors), npm run build, and Playwright (own port, CI=1) on ladder-config, ladder-account-selector, loading-fallback and ladder-timeline: 15/15 passed.

Body-stale:

• "Two plain-text loaders were still left": the ladder timeline's Loading complete retained timeline... (above) is a third in the same section.

_{Reviewer: rD1. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at 5d0c01a6e8e04a5a537d6e8c09e78b2e77560b6a merged onto main (529b6e38), together with #1444, #1446, #1449, #1450, #1451, #1457, #1458 in that order (combined tree 7fb727bb, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 1 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 1 approving independent review(s) on 5d0c01a6, green CI, and the combined-tree verification above. Squash-merged at exactly 5d0c01a6e8e04a5a537d6e8c09e78b2e77560b6a.