Fix Transaction Filter Sidebar: i18n crash, a11y landmarks, label sizing - #1409
Merged
emdevelopa merged 2 commits intoAug 28, 2026
Merged
Conversation
|
@chinelo002 is attempting to deploy a commit to the Emmanuel's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@chinelo002 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! 🚀 |
The new transactionFilters i18n keys added in the previous commit used
plain ASCII ("Busqueda", "descripcion", "Ate") while the rest of
es.json/pt.json consistently uses accented characters — fixed to match
(búsqueda/descripción, transações/até).
Also removes an unused FilterSyncIndicator import — dead since an
earlier commit (883297a) replaced it with the component's own
SyncSpinner, flagged by eslint's no-unused-vars.
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.
Summary
Investigated the Transaction Filter Sidebar (
TransactionFilterSidebar.tsx) for the 4 assigned UX issues and found a genuine, severe rendering bug underlying all of them, plus real a11y and typography gaps.The core bug (closes #1350 — incorrect state rendering)
The component was migrated to
next-intl(useTranslations("transactionFilters")) at some point, but:transactionFiltersnamespace was never added to any of the 3 locale files (messages/en.json,es.json,pt.json).NextIntlClientProvider.Since
NextIntlClientProviderinlayout.tsxloads messages viagetMessages()(the full locale file), and that file has notransactionFilterskey, every render of this component in the running app throws/falls back to raw message keys — this isn't a test-only issue, it's a production rendering bug. Confirmed via the existing test suite: 0/80 tests passed before this fix (all failed withuseTranslationscontext/missing-message errors), 80/80 pass after.Fix: added the full
transactionFiltersmessage tree (title, search/status/asset/date-range labels, sync-state hints, aria labels, pluralized "Clear all N active filters") to all 3 locale files, and fixed the test file to render through aNextIntlClientProviderwrapping the real English messages — matching how the app actually renders this component.Accessibility (closes #1348)
aria-label="Filter sidebar"was hardcoded English, never translated fores/ptlocales. Now reads from the samelandmarkLabeltranslation key as the desktop panel.role="complementary"landmark and mobilerole="dialog"to share one purpose-builtlandmarkLabel("Transaction filters") distinct from the visible heading ("Filters") — a screen reader user navigating by landmark now gets a more specific label than the generic on-screen heading text.SyncSpinnerhad a redundant innersr-onlyspan duplicating the same text already set via the parent'saria-label— screen readers usearia-labelonce set, so the inner span was dead weight, and it caused the visible "Clearing…" text next to the spinner to be announced twice. Removed the redundant node.container.querySelector('#' + hintId)) that silently broke onuseId()'s colon-containing ids — switched togetElementById, which needs no escaping.Typography (closes #1349)
Bumped the 4 primary field-group labels (Search, Status, Asset filter, Date range) from
10pxto11px— the smallest comfortably-readable size for uppercase-tracked label text. Scoped narrowly to this component's primary labels rather than the sub-10px pattern used elsewhere in the codebase (35+ other components share that convention; a codebase-wide type-scale pass is a separate, much larger change and out of scope here).UI revamp (closes #1351)
The i18n fix above is the substantive UI-facing change: previously the component's actual runtime behavior (crash/fallback) never matched what the polished JSX and test file describe. With working translations, the existing careful UX (optimistic filter state, staggered animations, sync indicators, dual-control-safe pending states) now actually renders as designed.
Test plan
npx vitest run src/components/TransactionFilterSidebar.test.tsx— 80/80 passing (was 0/80 before this fix)npx tsc --noEmit— zero new errors (pre-existing unrelated failures inOnboardingProgressTracker.tsx/PortfolioChartWidget.tsxconfirmed viagit diff upstream/mainto predate this change)npx eslinton both touched files — zero errors, only pre-existing warnings unrelated to this diffCloses #1348
Closes #1349
Closes #1350
Closes #1351