fix(ui-ux): expose bare loading text as live regions - #558
Merged
seonghobae merged 5 commits intoAug 24, 2026
Conversation
Async loading placeholders in App.tsx and FiveW1H.tsx rendered as
plain <p> text with no role/aria-live, so assistive tech never
announced the transition from "Loading..." to the resolved content.
This included the app-root auth gate every session passes through,
and the post-detail popup's loading paragraph.
Add role="status" to every bare loading <p> (19 in App.tsx, 1 in
FiveW1H.tsx), matching the role="status" pattern already used
elsewhere in the same files (e.g. "Loading posts...",
SourceResearchPanel). No behavior change beyond the attribute.
Tests: add an RTL assertion that the auth-loading gate and the
post-detail popup's loading state are exposed via getByRole("status")
rather than only getByText, extending stubBackend with a
deferPostOne option to deterministically observe the popup's
pre-resolution state.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011jWJzKUd82yy97esEBfJbt
|
Warning Review limit reachedNext included review available in 53 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
seonghobae
enabled auto-merge
August 24, 2026 01:26
- The unauthenticated Log in handler now calls returnUrlFromLocation() then rememberOidcReturnUrl() before signinRedirect, per ADR 0109, so a shared /?post= link still opens that post after enterprise SSO. The previously unused oidcReturnUrl import is now load-bearing. - AdminPanel renders only when accessToken is a string; the OIDC access token is string | undefined before isAuthenticated narrowing.
…egions # Conflicts: # frontend/src/App.test.tsx # frontend/src/App.tsx
seonghobae
enabled auto-merge
August 24, 2026 05:55
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Gap
Async loading/status text was rendered as plain
<p>text with norole/aria-live, in ~20 places acrossfrontend/src/App.tsxandfrontend/src/components/FiveW1H.tsx(this worktree's snapshot has a different line count than the original gap report, but the same underlying pattern). Assistive-tech users are never told when aLoading…placeholder is replaced by real content.Highest-traffic instances:
App.tsx— the app-root auth gate (if (auth.isLoading) return <p>{t("Loading authentication state...")}</p>) — every session passes through this before anything else renders.App.tsx— the post-detail popup's{!post && !error && <p>{t("Loading...")}</p>}— focus lands in the popup, but the loading→title transition is silent to AT.App.tsx—EventLineageSection's twoLoading lineage...returns, plus 15 more bare loading paragraphs across related panels (evidence, related nodes, evaluation, tickets, activity, affiliate tree, analysis runs, rankings, calendar, reports, customer master, related posts).FiveW1H.tsx—Loading 5W1H....This contrasts with the existing correct pattern already used a few components over, e.g.
App.tsx's<p role="status">{t("Loading posts...")}</p>for the board list.Fix
Added
role="status"to every bare loading<p>identified (19 inApp.tsx, 1 inFiveW1H.tsx), matching the existingrole="status"convention used elsewhere in the same files. These are plain transient text nodes with no retry action or extra description, so a rawrole="status"attribute is the smallest correct fix — no need to wrap them in a richer status component. No behavior change beyond the attribute; no new user-facing strings, so no new i18n keys needed.Tests
it("announces the app-root auth loading gate as a live region", ...)— setsmockAuth.isLoading = trueand assertsscreen.getByRole("status")carries theLoading authentication state...text (previously only reachable viagetByText).it("announces the post-detail popup loading state as a live region before the post resolves", ...)— extends the existingstubBackendhelper with adeferPostOneoption (mirroring the existingdeferMepattern) to deterministically observe the popup's pre-resolution state, then assertswithin(popup).getByRole("status")before asserting the loaded content appears and the loading text is gone.Checks run (from
frontend/)corepack pnpm exec tsc -b— same 2 pre-existing errors as on the unmodified base commit (App.tsx:104unused import,App.tsx:4623AdminPanelprop type), confirmed unrelated by diffing againstHEADbefore these edits. No new errors.corepack pnpm exec oxlint src/App.tsx src/App.test.tsx src/components/FiveW1H.tsx— same 2 pre-existing warnings (same unused import), no new warnings/errors.corepack pnpm exec vitest run src/App.test.tsx— 80 passed (80), including the 2 new tests.🤖 Generated with Claude Code
https://claude.ai/code/session_011jWJzKUd82yy97esEBfJbt