fix(a11y): announce loading state in LoadingSpinner for screen readers

[tarunsinghofficial]

Fixes #505 #405

Summary by CodeRabbit

• Accessibility

  • Loading spinner now features ARIA accessibility attributes and live region announcements, enabling screen readers to accurately communicate loading state to users. Includes full localization support with translated announcements across multiple languages.

• Tests

  • Test suite updated to comprehensively verify accessibility features function correctly across language localization and assistive technology scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

LoadingSpinner component now declares ARIA accessibility attributes (role="status", aria-live="polite", and an i18n-translated aria-label) to properly announce loading state to assistive technologies. Corresponding test updates verify the ARIA properties and localized visible text across locale changes, with a full mock of svelte-i18n.

Changes

LoadingSpinner Accessibility

Layer / File(s)	Summary
LoadingSpinner accessibility markup src/components/LoadingSpinner.svelte	The wrapper div adds role="status", aria-live="polite", and aria-label with a translated "loading" string to announce loading state to assistive technologies.
Accessibility tests with i18n mocking src/components/__tests__/LoadingSpinner.test.js	Test suite fully mocks svelte-i18n with locale and translator subscription; assertions verify the status role with aria-live="polite" and localized accessible names, and test locale changes from English to Spanish; prior untranslated i18n key checks and structure-specific assertions are removed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• Add aria-live announcements to loading spinners #405: Both changes add role="status", aria-live="polite", and a translated aria-label to a loading spinner component with the same code-level ARIA and translation updates.

Suggested reviewers

• aaronbrethorst

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: adding ARIA accessibility attributes to announce loading state in the LoadingSpinner component for screen readers.
Linked Issues check	✅ Passed	The PR implements aria-live, role, and aria-label attributes on the LoadingSpinner container, directly addressing issue #505's requirement to add aria-live to inline LoadingSpinner.
Out of Scope Changes check	✅ Passed	All changes are focused on LoadingSpinner accessibility: adding ARIA attributes to the component and updating corresponding tests to verify the new accessibility features.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/aria-load-spinners

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/components/LoadingSpinner.svelte (1)

2-2: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use the project i18n wrapper instead of importing svelte-i18n directly.

Please import translations from src/lib/i18n.js so this component stays on the repo’s centralized i18n initialization/fallback contract.

As per coding guidelines, "Use i18n via src/lib/i18n.js with English as synchronous fallback and other 24 locales lazy-loaded".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/LoadingSpinner.svelte` at line 2, Replace the direct import of
t from 'svelte-i18n' with the project's i18n wrapper by importing t from
'src/lib/i18n.js' so LoadingSpinner.svelte uses the centralized i18n
initialization/fallback; update the import statement that currently references
"t" to use the wrapper export and ensure the component continues to call t(...)
unchanged so English remains the synchronous fallback and other locales stay
lazy-loaded per the repo convention.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/components/LoadingSpinner.svelte`:
- Line 2: Replace the direct import of t from 'svelte-i18n' with the project's
i18n wrapper by importing t from 'src/lib/i18n.js' so LoadingSpinner.svelte uses
the centralized i18n initialization/fallback; update the import statement that
currently references "t" to use the wrapper export and ensure the component
continues to call t(...) unchanged so English remains the synchronous fallback
and other locales stay lazy-loaded per the repo convention.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: b28cf681-11c6-48b2-b2f3-e72a3d55a1a3

📥 Commits

Reviewing files that changed from the base of the PR and between d0a1b1c and 0422e86.

📒 Files selected for processing (2)

• src/components/LoadingSpinner.svelte
• src/components/__tests__/LoadingSpinner.test.js

[coveralls]

Coverage is 80.783% — fix/aria-load-spinners into develop. No base build found for develop.

[aaronbrethorst]

Nice, focused fix, Tarun. This does exactly what issue #505 asks for and nothing more: the inline LoadingSpinner now carries role="status", aria-live="polite", and a localized aria-label, so screen reader users finally get told something is happening. It mirrors the pattern already established in FullPageLoadingSpinner.svelte to the letter, which is exactly what I want to see — consistency over cleverness.

The test rewrite is the real star here. Swapping in a local svelte-i18n mock that actually flips en→es turns what could have been a tautological "the mock returns the key" check into a genuine reactivity test: it proves both the aria-label and the visible text re-render on a locale change. That's a meaningfully better test than what it replaced.

Critical Issues (0 found)

None.

Important Issues (0 found)

None. Tests pass (npx vitest run — 4/4 green), npm run lint is clean, and the accessible-name assertions hold up under scrutiny: removing the aria-label from the component would actually fail the Spanish test, since aria-label wins over text content for the computed accessible name. The coverage is real, not decorative.

Suggestions (1 found)

• The inner <svg> is purely decorative but isn't marked aria-hidden="true" [src/components/LoadingSpinner.svelte:12]. In practice the parent role="status" already owns the accessible name via aria-label, so the SVG contributes nothing to assistive tech and this is belt-and-suspenders. It also applies equally to the existing FullPageLoadingSpinner.svelte, so it's not a regression you introduced. Out of scope for this PR — tracked separately (see below) rather than gating the merge.

A note on the CodeRabbit comment

The automated suggestion to import t from a src/lib/i18n.js wrapper is a false alarm — i18n.js exports languages, getInitialLocale, and isRTL, but not t. Importing t directly from svelte-i18n is the established convention across the codebase (ArrivalDeparture, AlertsModal, SearchPane, and ~15 others). Your import is correct; no change needed.

Strengths

• Implementation is minimal and matches the sibling component exactly.
• Satisfies every acceptance-criteria checkbox in issue #505.
• Test mock is a legitimate, reusable fixture that makes locale reactivity genuinely testable.
• beforeEach(() => locale.set('en')) properly isolates the mutable mock state between tests.
• waitFor correctly handles the async reactivity flush after locale.set('es').

Recommended Action

Merge as-is. The single suggestion above is fit-and-finish and belongs in a follow-up, not this PR.