Skip to content

Fix/accessibility and perf frontend fixes - #1231

Closed
spotkorner-dot wants to merge 4 commits into
solutions-plug:mainfrom
spotkorner-dot:fix/accessibility-and-perf-frontend-fixes
Closed

Fix/accessibility and perf frontend fixes#1231
spotkorner-dot wants to merge 4 commits into
solutions-plug:mainfrom
spotkorner-dot:fix/accessibility-and-perf-frontend-fixes

Conversation

@spotkorner-dot

Copy link
Copy Markdown

closes #1168
closes #1169
closes #1170
closes #1171

Description

Type of Change

  • Bug fix
  • New feature
  • Refactor / code cleanup
  • Documentation update
  • CI / tooling change
  • Breaking change

Testing Done

Bundle Size

Chunk Before After
vendor.js
main*.js
pages/_app*.js

Checklist

  • Tests pass locally
  • Documentation updated (if applicable)
  • No breaking changes, or breaking changes are documented above
  • If you added or changed an API endpoint, regenerated the OpenAPI spec and committed the result:
    cd services/api && cargo run --bin generate-openapi > openapi.yaml
    git add openapi.yaml && git commit -m "chore: regenerate openapi.yaml"
  • If you changed system architecture (new service, database, external dependency, or network boundary), updated docs/architecture.md
  • Bundle size checked (if frontend changes)

Related Issues

Closes #

The email input only ever referenced email-error via aria-describedby,
so screen-reader users navigating back to the field after an API
failure got no indication of the associated error.
setLocale updated in-memory/localStorage state and re-rendered text but
never touched the html lang attribute, so switching locales would leave
the page's declared language mismatched with its visible content
(WCAG 3.1.1).
Locale was hand-typed as 'en' | 'es' | 'fr' | 'de' while translations
only implemented 'en', so the language selector's type promised locales
it could never actually render. Locale is now keyof typeof translations,
so it can't drift ahead of what's implemented, and the selector's
rendered options are guaranteed to match.
client.ts (imported by LandingPage.tsx and Statistics.tsx) shipped the
full ~50-entry CONTRACT_ERROR_MESSAGES map and 15+ admin/blockchain/email
request builders, even though the landing page only ever calls
getStatistics and newsletterSubscribe.

Split into:
 - request.ts: shared retry/timeout/cache request layer
 - client.ts: the two endpoints the landing page actually uses
 - admin-client.ts: everything else (admin, blockchain, email, contract
   error messages), for whatever internal tooling needs that surface

A production build shows the two clients' code and string literals no
longer appear together; client.ts now type-checks to expose only
getStatistics and newsletterSubscribe, enforced by a test.

Note: a byte-for-byte comparison of the built .next/static/chunks output
against main showed no size difference for this app today, because
Turbopack's dead-code elimination already stripped the unused api
object's properties before this split. bundlewatch.config.json also
still targets classic webpack output paths (vendor.js, main*.js,
pages/_app*.js) that don't exist under this app's Turbopack/App Router
build, so it isn't actually wired up to catch a regression here either.
The split is still correct to make explicit and guards against future
regressions (e.g. a stray import pulling the admin surface back in), but
the bundlewatch config would need a separate fix to point at real
Turbopack chunk paths before it could verify this in CI.
@drips-wave

drips-wave Bot commented Jul 26, 2026

Copy link
Copy Markdown

@spotkorner-dot Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

hman38705 added a commit that referenced this pull request Jul 28, 2026
- Align react to 19.2.8 to match react-dom (mismatch from an earlier
  dependabot merge that bumped only react-dom, breaking every React
  test suite).
- Port #1231's non-superseded pieces now that #1241 (not #1231) is the
  chosen API-client-split design: LandingPage aria-describedby wiring
  to the API-error message, and useI18n's document.documentElement.lang
  sync effect.
- Fix six test files still importing `api` from the legacy
  lib/api/client.ts barrel (re-exports admin-client's api) while
  LandingPage/Statistics now import from public-client.ts — the mocks
  were silently not applying, letting real fetch calls leak through
  and break the newsletter-signup mock in the same test.

Copy link
Copy Markdown
Contributor

Closing in favor of #1241 for the API-client-split design (the two PRs proposed incompatible module layouts for the same split — #1241 was chosen since it was already stacked on #1235/#1240's fixes). #1231's other independent pieces (LandingPage aria-describedby wiring, useI18n document.lang sync effect) have been ported and merged separately via #1254.

@hman38705 hman38705 closed this Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment