feat(reports): shared FilterPanel component (temporal slot + account multi-select) #282

Closed

maximus wants to merge 1 commit from issue-274-filter-panel into issue-273-account-filter-services

maximus commented

2026-07-11 19:32:19 +00:00

Owner

Generated autonomously by /autopilot run of 2026-07-11

Resolves #274

Summary

Adds the shared <FilterPanel> component (src/components/reports/FilterPanel.tsx) for the report pages:

Receives the page's own temporal control as a temporalControl: ReactNode prop and renders it as-is (no temporalMode enum — rejected during planning to avoid mixing state sources; each page keeps owning its own temporal state).
Renders a checkbox multi-select over accounts: ImportSource[] (import sources), controlled via accountIds: number[] + onAccountIdsChange. Empty accountIds = no filter (all sources).
Selection toggling extracted as a pure, exported toggleAccountId(accountIds, id) helper for direct unit testing.
i18n FR/EN under reports.filters.accounts.{label,hint}. Copy deliberately says "sources"/"sources d'import" everywhere (mirroring the existing transactions.filters.allSources vocabulary), never "comptes"/"accounts", so the filter reads as distinct from the Bilan module's balance_accounts ("Compte"/"Account" throughout that module's own UI) — this was an explicit acceptance criterion.
Display-only, no business logic beyond selection wiring; does not fetch its own data (accounts/accountIds/onAccountIdsChange all come from the parent).

Scope is intentionally tight per the issue: this creates the standalone shared component only. Wiring it into the actual report pages (Trends/ByCategory/Compare/Budget) is left to follow-up issues #275/#276.

Testing approach note (caveat)

This project has no @testing-library/react/jsdom configured (see PriceFetchControl.test.tsx's precedent, which bypasses rendering entirely and re-simulates logic inline). Since FilterPanel has no internal state/hooks besides useTranslation (mocked to a plain function in the test), the test file calls the component directly as a plain function and walks the returned React-element tree (a plain object graph — JSX creation never touches the DOM) to assert render output and simulate checkbox clicks by invoking .props.onChange() directly. This exercises the real shipped component code rather than re-implementing its logic in the test, which is a stronger form of coverage than the established precedent while still requiring no jsdom.

Decisions (MEDIUM ambiguity, noted per instructions)

Default checkbox state when accountIds is empty: all checkboxes render unchecked (no accounts explicitly selected = no active filter = all accounts shown). Checking boxes progressively narrows the filter to just the checked ids; unchecking the last one returns to []. This is the literal, lowest-risk reading of the acceptance criterion ("empty selection → accountIds = [] = all") and needs no extra pseudo-"All" toggle state.
- Rejected alternative: all-checked-by-default ("everything included, uncheck to exclude") — more common for some multi-select UIs, but it inverts what "empty" means (would make an all-unchecked state read as "show nothing", contradicting the criterion) and would need extra state to reconcile. Left as a follow-up if product feedback wants it.
Added a small always-visible hint (reports.filters.accounts.hint, i18n'd) clarifying "no selection = all sources", since the checkbox-list has no explicit "All" affordance.

Test plan

npm run build (tsc + vite) green
npm test — 773 �� 788 (added 15, zero regressions)
CHANGELOG entries added (EN + FR, ### Added)
i18n keys added in both en.json/fr.json

Generated autonomously by /autopilot run of 2026-07-11 Resolves #274 ## Summary Adds the shared `<FilterPanel>` component (`src/components/reports/FilterPanel.tsx`) for the report pages: - Receives the page's own temporal control as a `temporalControl: ReactNode` prop and renders it as-is (no `temporalMode` enum — rejected during planning to avoid mixing state sources; each page keeps owning its own temporal state). - Renders a checkbox multi-select over `accounts: ImportSource[]` (import sources), controlled via `accountIds: number[]` + `onAccountIdsChange`. Empty `accountIds` = no filter (all sources). - Selection toggling extracted as a pure, exported `toggleAccountId(accountIds, id)` helper for direct unit testing. - i18n FR/EN under `reports.filters.accounts.{label,hint}`. Copy deliberately says "sources"/"sources d'import" everywhere (mirroring the existing `transactions.filters.allSources` vocabulary), never "comptes"/"accounts", so the filter reads as distinct from the Bilan module's `balance_accounts` ("Compte"/"Account" throughout that module's own UI) — this was an explicit acceptance criterion. - Display-only, no business logic beyond selection wiring; does not fetch its own data (accounts/accountIds/onAccountIdsChange all come from the parent). Scope is intentionally tight per the issue: this creates the standalone shared component only. Wiring it into the actual report pages (Trends/ByCategory/Compare/Budget) is left to follow-up issues #275/#276. ## Testing approach note (caveat) This project has no `@testing-library/react`/jsdom configured (see `PriceFetchControl.test.tsx`'s precedent, which bypasses rendering entirely and re-simulates logic inline). Since `FilterPanel` has no internal state/hooks besides `useTranslation` (mocked to a plain function in the test), the test file calls the component directly as a plain function and walks the returned React-element tree (a plain object graph — JSX creation never touches the DOM) to assert render output and simulate checkbox clicks by invoking `.props.onChange()` directly. This exercises the real shipped component code rather than re-implementing its logic in the test, which is a stronger form of coverage than the established precedent while still requiring no jsdom. ## Decisions (MEDIUM ambiguity, noted per instructions) - **Default checkbox state when `accountIds` is empty**: all checkboxes render **unchecked** (no accounts explicitly selected = no active filter = all accounts shown). Checking boxes progressively narrows the filter to just the checked ids; unchecking the last one returns to `[]`. This is the literal, lowest-risk reading of the acceptance criterion ("empty selection → accountIds = [] = all") and needs no extra pseudo-"All" toggle state. - *Rejected alternative*: all-checked-by-default ("everything included, uncheck to exclude") — more common for some multi-select UIs, but it inverts what "empty" means (would make an all-unchecked state read as "show nothing", contradicting the criterion) and would need extra state to reconcile. Left as a follow-up if product feedback wants it. - Added a small always-visible hint (`reports.filters.accounts.hint`, i18n'd) clarifying "no selection = all sources", since the checkbox-list has no explicit "All" affordance. ## Test plan - [x] `npm run build` (tsc + vite) green - [x] `npm test` — 773 �� 788 (added 15, zero regressions) - [x] CHANGELOG entries added (EN + FR, `### Added`) - [x] i18n keys added in both `en.json`/`fr.json`

maximus added 1 commit 2026-07-11 19:32:20 +00:00

feat(reports): shared FilterPanel component (temporal slot + account multi-select) 70b2cd2e42

Adds src/components/reports/FilterPanel.tsx, the shared filter bar for the
report pages. It renders the page's own temporal control (PeriodSelector /
CompareReferenceMonthPicker / YearNavigator / …) as-is via a `temporalControl`
ReactNode prop, plus a checkbox multi-select over `accounts: ImportSource[]`.
No `temporalMode` enum — that design was rejected to avoid mixing state
sources; the panel owns no temporal state at all.

Empty `accountIds` means no filter (all sources). Selection toggling is a
pure, exported `toggleAccountId` helper. Copy says "sources"/"import
sources" everywhere, never "comptes"/"accounts", so the filter reads as
distinct from the Bilan module's own "Compte" vocabulary.

This issue creates the standalone component only; wiring it into report
pages is left to follow-up issues (#275/#276).

Resolves #274

maximus added the

autopilot:pending-human

label 2026-07-11 19:32:34 +00:00

maximus commented

2026-07-11 20:20:47 +00:00

Author

Owner

`/pr-review` — PR #282 · `<FilterPanel>` partagé (slot temporel + multi-select comptes)

Verdict : APPROVE

Résumé

Composant d'affichage partagé propre, contrôlé (aucune donnée fetchée en propre), au scope volontairement serré : le composant standalone seul, le câblage dans les pages reste pour #275/#276. Bien testé, conventions respectées, aucun blocage.

Vérifications

Types — ImportSource (src/shared/types/index.ts) correspond exactement aux fixtures du test ; accountIds: number[] cohérent avec la fondation posée en #272/#273.
i18n — clés reports.filters.accounts.{label,hint} ajoutées dans les deux locales (en.json/fr.json), placées sous le bon namespace reports.filters ; aucun texte en dur, tout passe par t().
Vocabulaire — « sources » / « sources d'import » partout, jamais « comptes »/« accounts » (critère d'acceptation explicite pour distinguer du module Bilan balance_accounts). Aligné sur le vocabulaire existant transactions.filters.allSources.
Conventions UI — carte bg-[var(--card)] rounded-xl border border-[var(--border)] et accent-[var(--primary)] sur la checkbox conformes aux composants reports existants (CategoryZoomHeader:63, cartes de charts).
A11y — role="group" + aria-label (pattern déjà utilisé par ViewModeToggle/TrendsChartTypeToggle), checkboxes enveloppées dans un <label> (association implicite, cible cliquable correcte).
Logique pure — toggleAccountId immuable, ordre préservé au retrait, testé y compris la non-mutation. Câblage checked/onChange correct.
Tests — 15 tests (5 toggleAccountId + 7 render + 3 wiring), aucun .only/.skip, cohérent avec le compte annoncé 773→788. L'approche (appel direct du composant comme fonction pure + walk de l'arbre d'éléments) exerce le vrai code livré plutôt que d'en ré-implémenter la logique — plus fort que le précédent PriceFetchControl.test.tsx.
CHANGELOG — entrées EN + FR sous ### Added, référence #274.
Commit — format conventionnel (feat(reports): …).
Sécurité / data — composant d'affichage pur : pas de secret, pas de SQL, pas de migration, pas de surface d'injection.

Suggestions (non bloquantes)

Test « multi-select » (FilterPanel.test.tsx:191) — le test « checking a second account preserves the first » rend le composant une seule fois avec accountIds: [], donc les deux handlers onChange capturent le même [] figé ; les assertions [1] puis [2] sont correctes pour cette simulation, mais le test ne prouve pas l'accumulation [1]→[1,2] (qui exigerait un re-render avec la prop mise à jour). L'accumulation est bien couverte par les tests purs toggleAccountId([1,2],3)=[1,2,3] — le nom du test surestime juste ce qu'il vérifie.
État accounts vide — quand la liste est vide, le panneau affiche quand même le label « Sources d'import », l'icône et le hint, sans aucune case. Le câblage (#275/#276) pourrait masquer le bloc comptes si accounts.length === 0.
Hint permanent — « Aucune sélection = toutes les sources » reste affiché même quand une sélection est active (légèrement redondant). Optionnel : ne l'afficher que si accountIds.length === 0.
Icône Inbox — connote habituellement un état vide ; une icône type filtre/entrée de fichier lirait peut-être plus clairement « sources d'import ». Purement cosmétique.
Fragilité du test — l'appel direct FilterPanel(props) casserait si le composant gagnait plus tard un vrai hook d'état (useState/useEffect). Documenté dans le commentaire, acceptable — à garder en tête pour les PRs de câblage si le composant devient stateful.

Note pile stackée

CI check.yml ne se déclenche pas sur ce maillon (base = issue-273-…, pas main) — il affiche no-status. Valider le tip cumulé en local (tsc && vite build, vitest run, cargo check/test) avant de merger la pile #272→#276.

## `/pr-review` — PR #282 · `<FilterPanel>` partagé (slot temporel + multi-select comptes) ### Verdict : APPROVE ### Résumé Composant d'affichage partagé propre, contrôlé (aucune donnée fetchée en propre), au scope volontairement serré : le composant standalone seul, le câblage dans les pages reste pour #275/#276. Bien testé, conventions respectées, aucun blocage. ### Vérifications - **Types** — `ImportSource` (`src/shared/types/index.ts`) correspond exactement aux fixtures du test ; `accountIds: number[]` cohérent avec la fondation posée en #272/#273. - **i18n** — clés `reports.filters.accounts.{label,hint}` ajoutées dans **les deux** locales (`en.json`/`fr.json`), placées sous le bon namespace `reports.filters` ; aucun texte en dur, tout passe par `t()`. - **Vocabulaire** — « sources » / « sources d'import » partout, jamais « comptes »/« accounts » (critère d'acceptation explicite pour distinguer du module Bilan `balance_accounts`). Aligné sur le vocabulaire existant `transactions.filters.allSources`. - **Conventions UI** — carte `bg-[var(--card)] rounded-xl border border-[var(--border)]` et `accent-[var(--primary)]` sur la checkbox conformes aux composants reports existants (`CategoryZoomHeader:63`, cartes de charts). - **A11y** — `role="group"` + `aria-label` (pattern déjà utilisé par `ViewModeToggle`/`TrendsChartTypeToggle`), checkboxes enveloppées dans un `<label>` (association implicite, cible cliquable correcte). - **Logique pure** — `toggleAccountId` immuable, ordre préservé au retrait, testé y compris la non-mutation. Câblage `checked`/`onChange` correct. - **Tests** — 15 tests (5 `toggleAccountId` + 7 render + 3 wiring), aucun `.only`/`.skip`, cohérent avec le compte annoncé 773→788. L'approche (appel direct du composant comme fonction pure + walk de l'arbre d'éléments) exerce le vrai code livré plutôt que d'en ré-implémenter la logique — plus fort que le précédent `PriceFetchControl.test.tsx`. - **CHANGELOG** — entrées EN + FR sous `### Added`, référence #274. - **Commit** — format conventionnel (`feat(reports): …`). - **Sécurité / data** — composant d'affichage pur : pas de secret, pas de SQL, pas de migration, pas de surface d'injection. ### Suggestions (non bloquantes) 1. **Test « multi-select » (`FilterPanel.test.tsx:191`)** — le test *« checking a second account preserves the first »* rend le composant **une seule fois** avec `accountIds: []`, donc les deux handlers `onChange` capturent le même `[]` figé ; les assertions `[1]` puis `[2]` sont correctes pour cette simulation, mais le test ne prouve pas l'**accumulation** `[1]→[1,2]` (qui exigerait un re-render avec la prop mise à jour). L'accumulation est bien couverte par les tests purs `toggleAccountId([1,2],3)=[1,2,3]` — le nom du test surestime juste ce qu'il vérifie. 2. **État `accounts` vide** — quand la liste est vide, le panneau affiche quand même le label « Sources d'import », l'icône et le hint, sans aucune case. Le câblage (#275/#276) pourrait masquer le bloc comptes si `accounts.length === 0`. 3. **Hint permanent** — « Aucune sélection = toutes les sources » reste affiché même quand une sélection est active (légèrement redondant). Optionnel : ne l'afficher que si `accountIds.length === 0`. 4. **Icône `Inbox`** — connote habituellement un état vide ; une icône type filtre/entrée de fichier lirait peut-être plus clairement « sources d'import ». Purement cosmétique. 5. **Fragilité du test** — l'appel direct `FilterPanel(props)` casserait si le composant gagnait plus tard un vrai hook d'état (`useState`/`useEffect`). Documenté dans le commentaire, acceptable — à garder en tête pour les PRs de câblage si le composant devient stateful. ### Note pile stackée CI `check.yml` ne se déclenche pas sur ce maillon (base = `issue-273-…`, pas `main`) — il affiche `no-status`. Valider le **tip cumulé en local** (`tsc && vite build`, `vitest run`, `cargo check/test`) avant de merger la pile #272→#276.

maximus commented

2026-07-11 20:39:45 +00:00

Author

Owner

Merged into main via local fast-forward (commit fe9ae01) as a linear stack. Forgejo doesn't flag non-API merges as merged; closing manually.

Merged into `main` via local fast-forward (commit fe9ae01) as a linear stack. Forgejo doesn't flag non-API merges as merged; closing manually.

maximus closed this pull request

2026-07-11 20:39:46 +00:00