Skip to content

fix(ui): expose DataTable loading semantics and hide the decorative spinner - #11869

Merged
diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
pacocartones:fix/data-table-loading-a11y
Aug 28, 2026
Merged

diegosouzapw merged 2 commits into
cheaper-inference:release/v3.8.51from
pacocartones:fix/data-table-loading-a11y

Conversation

@pacocartones

@pacocartones pacocartones commented Aug 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

DataTable's loading early-return renders a bare <div> containing an animated ⏳ emoji
followed by the localized loading label. The emoji carries no aria-hidden, so assistive
technology reads the decorative glyph as content immediately ahead of the label, and the
container advertises neither a status role nor aria-busy.

This PR mirrors PageLoading in src/shared/components/Loading.tsx — the repo's own
convention for a loading surface: role="status", aria-live="polite" and
aria-busy="true" on the container, aria-hidden="true" on the decorative glyph.

Scope, stated honestly. The guaranteed, user-visible win here is (a): the decorative
glyph stops being announced as noise before the label. (b) is correct, future-proof
semantic labelling — it is not an announcement fix in the tree as it stands today.
All three call sites that pass loading (ComplianceTab, and the two ConductorPageClient
tables) enter the loading state only on mount and never re-enter it — setLoading(false)
is never followed by setLoading(true), and snapshot is never reset to null — so
aria-live has no transition to announce right now. It starts announcing the moment any
consumer re-enters the loading state (a refresh button, a poll that clears its snapshot),
and until then it costs nothing and keeps the primitive consistent with PageLoading.

No i18n keys were added: common.loading and common.noData already exist. Layout,
animation, keyboard behaviour, the empty state and the public prop contract are untouched.

Changelog fragment: changelog.d/fixes/11869-data-table-loading-a11y.md.

Related Issues

Validation

  • Change type: UI
  • Focused tests and category gates from the golden path
  • npm run lint
  • Reconciled with the current active release base (branched from release/v3.8.51 @ dd8575e); focused checks rerun afterward
  • Production-code changes include a new or updated automated test in this PR
  • Changelog fragment added under changelog.d/fixes/ (npm run check:changelog-integrity → exit 0)

Test output

Red — new test run against the unpatched component (git checkout HEAD~1 -- src/shared/components/DataTable.tsx):

$ npx vitest run tests/unit/ui/data-table-loading-a11y.test.tsx

 RUN  v4.1.11 /data/tmp/oss047-pub/OmniRoute

 ❯ tests/unit/ui/data-table-loading-a11y.test.tsx (8 tests | 5 failed) 406ms
     × marks the loading container as a busy status region 189ms
     × hides the decorative spinner glyph from assistive technology 30ms
     × leaves only the loading label as readable text in the status region 17ms
     × keeps the spinner animation and layout untouched 10ms
     × prefers the loading state over the empty state 11ms

 Test Files  1 failed (1)
      Tests  5 failed | 3 passed (8)
exit code 1

Green — new test plus the existing keyboard test, with the fix applied:

$ npx vitest run tests/unit/ui/data-table-loading-a11y.test.tsx tests/unit/ui/data-table-keyboard-activation.test.tsx

 RUN  v4.1.11 /data/tmp/oss047-pub/OmniRoute


 Test Files  2 passed (2)
      Tests  14 passed (14)
   Start at  07:53:01
   Duration  2.72s (transform 290ms, setup 371ms, import 139ms, tests 193ms, environment 3.71s)

exit code 0

Lint (repo-wide) and format:

$ npm run lint

> omniroute@3.8.51 lint
> eslint . --cache --cache-location .eslintcache --suppressions-location config/quality/eslint-suppressions.json

exit code 0

$ npx eslint src/shared/components/DataTable.tsx tests/unit/ui/data-table-loading-a11y.test.tsx
exit code 0

$ npx prettier --check src/shared/components/DataTable.tsx tests/unit/ui/data-table-loading-a11y.test.tsx
Checking formatting...
All matched files use Prettier code style!
exit code 0

Tests Added Or Updated

Coverage Notes

src/shared/components/DataTable.tsx is the only production file touched, and only its
loading early-return. The new test file covers that branch directly — status role,
aria-live, aria-busy, the hidden glyph, the readable text left in the region, and the
preserved inline flex styling / @keyframes spin block — and also asserts the other two
branches (empty state, populated table) do not gain a status region or aria-busy.
The keyboard branch remains covered by the existing test. Coverage on the touched file
moves up, not down.

Reviewer Notes

  • The strongest claim this PR makes is "the decorative emoji is no longer read out"; the
    live-region wiring is deliberately framed as correct labelling rather than as an
    announcement, because nothing in the current tree re-enters the loading state. Please
    hold the description to that standard.
  • <style>{@Keyframes spin ...}</style> now lives inside the role="status" element.
    A <style> element contributes no text to the accessibility tree, so it does not affect
    the region's accessible content; the readable-text assertion in the test excludes it
    explicitly so the behaviour is pinned rather than assumed.
  • The empty-state emptyIcon glyph is likewise decorative and unlabelled, but changing it
    is outside this PR's scope and would touch a different branch and different consumer
    expectations. Happy to follow up separately if you want it.
  • No migrations, no feature flags, no manual validation needed.

…pinner

The DataTable loading early-return rendered a bare <div> with an animated
'hourglass' emoji followed by the localized loading label. The emoji carries
no aria-hidden, so assistive technology reads it as content ahead of the
label, and the container advertised neither a status role nor aria-busy.

Mirror PageLoading in src/shared/components/Loading.tsx: role="status",
aria-live="polite", aria-busy="true" on the container and aria-hidden on the
decorative glyph. Layout, animation, keyboard behaviour, the empty state and
the consumer contracts are untouched.
@diegosouzapw
diegosouzapw merged commit 5c0b68e into cheaper-inference:release/v3.8.51 Aug 28, 2026
7 of 16 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…pinner (cheaper-inference#11869)

DataTable's loading state now mirrors PageLoading's a11y convention (role=status, aria-live=polite, aria-busy=true on the container, aria-hidden on the decorative glyph) instead of announcing a bare emoji as content to assistive tech. Honest scope note in the PR body about when aria-live actually fires today. Thanks!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants