fix(charges): name forgotten bills in the alert + compact add-form - #230
Merged
Conversation
@Thierry feedback) 1. The forgotten-bills alert was blind ('1 facture de juin' — which one??). It now NAMES them: 'Jamais cochée en juin : Taxe voiture — vérifie qu'elle a bien été payée.' New generic unpaidChargesForPeriod returns the charges themselves (count derives from it); the card takes labels. 2. The add-charge form fields were oversized full-width stacked rows: label + amount now share one row on ≥md, and the CadenceField controls (frequency / day / anchor) collapse onto a single 3-column row on ≥md. Mobile keeps the vertical stack. i18n forgottenAlert v2 x5 ({labels} + singular/plural agreement). 76 tests green across the four affected suites.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Guide du relecteurImplémente deux changements UX pour les prélèvements : une mise en page compacte pour le formulaire d’ajout de prélèvement et le sélecteur de cadence, ainsi qu’une alerte de « factures oubliées » plus informative qui mentionne désormais les prélèvements impayés, soutenue par un petit refactor de domaine pour exposer les prélèvements impayés eux‑mêmes. Diagramme de séquence pour le flux mis à jour de l’alerte de factures oubliéessequenceDiagram
participant DashboardPage
participant unpaidChargesForPeriod
participant ProchainesFacturesCard
DashboardPage->>unpaidChargesForPeriod: unpaidChargesForPeriod(snapshot.charges, previousMonthPaidChargeIds, previousPeriod)
unpaidChargesForPeriod-->>DashboardPage: unpaidCharges
DashboardPage->>DashboardPage: forgottenLabels = unpaidCharges.map(label)
DashboardPage->>ProchainesFacturesCard: ProchainesFacturesCard({ forgottenLabels, forgottenMonthLabel })
ProchainesFacturesCard->>ProchainesFacturesCard: [forgottenLabels.length > 0]
ProchainesFacturesCard->>ProchainesFacturesCard: t('forgottenAlert', { count, month, labels })
ProchainesFacturesCard-->>DashboardPage: renders named forgotten bills alert
Modifications par fichier
Conseils et commandesInteragir avec Sourcery
Personnaliser votre expérienceAccédez à votre dashboard pour :
Obtenir de l’aide
Original review guide in EnglishReviewer's GuideImplements two UX changes for charges: a compact layout for the add-charge form and cadence selector, plus a more informative "forgotten bills" alert that now names the unpaid charges, backed by a small domain refactor to expose the unpaid charges themselves. Sequence diagram for the updated forgotten bills alert flowsequenceDiagram
participant DashboardPage
participant unpaidChargesForPeriod
participant ProchainesFacturesCard
DashboardPage->>unpaidChargesForPeriod: unpaidChargesForPeriod(snapshot.charges, previousMonthPaidChargeIds, previousPeriod)
unpaidChargesForPeriod-->>DashboardPage: unpaidCharges
DashboardPage->>DashboardPage: forgottenLabels = unpaidCharges.map(label)
DashboardPage->>ProchainesFacturesCard: ProchainesFacturesCard({ forgottenLabels, forgottenMonthLabel })
ProchainesFacturesCard->>ProchainesFacturesCard: [forgottenLabels.length > 0]
ProchainesFacturesCard->>ProchainesFacturesCard: t('forgottenAlert', { count, month, labels })
ProchainesFacturesCard-->>DashboardPage: renders named forgotten bills alert
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - j’ai identifié 1 point, et laissé quelques retours de haut niveau :
- L’alerte sur les factures oubliées concatène actuellement les libellés avec une simple virgule suivie d’un espace, ce qui n’est peut‑être pas idéal pour toutes les locales ; envisagez de déléguer la mise en forme de la liste à l’i18n (par exemple, un formateur de liste sensible à la locale ou un template) plutôt que de construire la chaîne directement dans le composant.
- Dans
CadenceField, la suppression des contraintesmin-w-32 flex-1sur les sélecteurs du jour et du mois peut entraîner des mises en page trop serrées ou des retours à la ligne pour des libellés traduits longs à des largeurs moyennes ; il pourrait être utile de réintroduire une largeur minimale ou un comportement flex pour garder les trois contrôles lisibles dans toutes les locales.
Prompt pour les agents IA
Veuillez traiter les commentaires de cette revue de code :
## Commentaires généraux
- L’alerte sur les factures oubliées concatène actuellement les libellés avec une simple virgule suivie d’un espace, ce qui n’est peut‑être pas idéal pour toutes les locales ; envisagez de déléguer la mise en forme de la liste à l’i18n (par exemple, un formateur de liste sensible à la locale ou un template) plutôt que de construire la chaîne directement dans le composant.
- Dans `CadenceField`, la suppression des contraintes `min-w-32 flex-1` sur les sélecteurs du jour et du mois peut entraîner des mises en page trop serrées ou des retours à la ligne pour des libellés traduits longs à des largeurs moyennes ; il pourrait être utile de réintroduire une largeur minimale ou un comportement flex pour garder les trois contrôles lisibles dans toutes les locales.
## Commentaires individuels
### Commentaire 1
<location path="src/components/dashboard/ProchainesFacturesCard.tsx" line_range="26-28" />
<code_context>
+ * previous period's ledger.
*/
- forgottenCount?: number;
+ forgottenLabels?: readonly string[];
/** Localized label of the previous month (e.g. « juin »), for the alert copy. */
forgottenMonthLabel?: string;
};
</code_context>
<issue_to_address>
**suggestion:** Clarifier le comportement de `forgottenMonthLabel` lorsqu’il est absent alors que des libellés sont présents.
Actuellement, `forgottenMonthLabel` peut être une chaîne vide tandis que `forgottenLabels` est non vide, ce qui peut produire une alerte avec un nom de mois manquant. Veuillez soit rendre `forgottenMonthLabel` obligatoire dès que `forgottenLabels.length > 0`, soit afficher l’alerte uniquement lorsque les deux sont présents, afin d’éviter un texte dégradé dans ce cas limite.
Implémentation suggérée :
```typescript
* Labels of the bills due LAST month that were never ticked (the per-period
* ledger resets naturally on month rollover, hiding them). Non-empty renders
* the "forgotten bills" alert NAMING the bills so the user checks they were
* actually paid. Computed upstream via `unpaidChargesForPeriod` on the
* previous period's ledger.
*
* The alert copy is only rendered when both `forgottenLabels` is non-empty
* and `forgottenMonthLabel` is present, to avoid degraded copy where the
* month name would be missing.
*/
forgottenLabels?: readonly string[];
/** Localized label of the previous month (e.g. « juin »), for the alert copy. */
forgottenMonthLabel?: string;
};
```
```typescript
payments,
todayIso,
locale,
forgottenLabels = [],
forgottenMonthLabel,
}: Props) {
```
Pour implémenter complètement le comportement demandé, le JSX qui affiche l’alerte "forgotten bills" (juste en dessous du commentaire `Forgotten-bills alert — factual, calm, FSMA-safe`) doit être mis à jour pour que l’alerte ne soit montrée que lorsque LES libellés et le mois sont tous deux présents, par exemple :
```tsx
const hasForgottenBillsAlert =
forgottenLabels.length > 0 && !!forgottenMonthLabel;
// ...
{hasForgottenBillsAlert && (
// existing alert JSX, which can safely use `forgottenMonthLabel`
)}
```
Cela garantit que nous ne rendons jamais un texte avec un libellé de mois manquant, même si `forgottenLabels` est non vide. Vous devrez intégrer `hasForgottenBillsAlert` (ou une condition équivalente inline) dans le composant/markup de l’alerte dans cette section.
</issue_to_address>Sourcery est gratuit pour l’open source – si nos revues vous plaisent, pensez à les partager ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The forgotten-bills alert currently concatenates labels with a simple comma/space, which may not be ideal for all locales; consider delegating list formatting to i18n (e.g. locale-aware list formatter or template) rather than building the string in the component.
- In
CadenceField, removing themin-w-32 flex-1constraints on the day and month selects may cause cramped layouts or wrapping for long translated labels at medium widths; it could be worth reintroducing some minimum width or flex behavior to keep the three controls readable across locales.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The forgotten-bills alert currently concatenates labels with a simple comma/space, which may not be ideal for all locales; consider delegating list formatting to i18n (e.g. locale-aware list formatter or template) rather than building the string in the component.
- In `CadenceField`, removing the `min-w-32 flex-1` constraints on the day and month selects may cause cramped layouts or wrapping for long translated labels at medium widths; it could be worth reintroducing some minimum width or flex behavior to keep the three controls readable across locales.
## Individual Comments
### Comment 1
<location path="src/components/dashboard/ProchainesFacturesCard.tsx" line_range="26-28" />
<code_context>
+ * previous period's ledger.
*/
- forgottenCount?: number;
+ forgottenLabels?: readonly string[];
/** Localized label of the previous month (e.g. « juin »), for the alert copy. */
forgottenMonthLabel?: string;
};
</code_context>
<issue_to_address>
**suggestion:** Clarify how `forgottenMonthLabel` behaves when missing while labels are present.
Currently `forgottenMonthLabel` can be an empty string while `forgottenLabels` is non-empty, which may produce an alert with a missing month name. Please either require `forgottenMonthLabel` whenever `forgottenLabels.length > 0`, or conditionally render the alert only when both are present to avoid degraded copy in that edge case.
Suggested implementation:
```typescript
* Labels of the bills due LAST month that were never ticked (the per-period
* ledger resets naturally on month rollover, hiding them). Non-empty renders
* the "forgotten bills" alert NAMING the bills so the user checks they were
* actually paid. Computed upstream via `unpaidChargesForPeriod` on the
* previous period's ledger.
*
* The alert copy is only rendered when both `forgottenLabels` is non-empty
* and `forgottenMonthLabel` is present, to avoid degraded copy where the
* month name would be missing.
*/
forgottenLabels?: readonly string[];
/** Localized label of the previous month (e.g. « juin »), for the alert copy. */
forgottenMonthLabel?: string;
};
```
```typescript
payments,
todayIso,
locale,
forgottenLabels = [],
forgottenMonthLabel,
}: Props) {
```
To fully implement the behavior you requested, the JSX that renders the "forgotten bills" alert (right below the `Forgotten-bills alert — factual, calm, FSMA-safe` comment) should be updated so that the alert is only shown when BOTH the labels and month are present, e.g.:
```tsx
const hasForgottenBillsAlert =
forgottenLabels.length > 0 && !!forgottenMonthLabel;
// ...
{hasForgottenBillsAlert && (
// existing alert JSX, which can safely use `forgottenMonthLabel`
)}
```
This ensures we never render copy with a missing month label even if `forgottenLabels` is non-empty. You’ll need to integrate `hasForgottenBillsAlert` (or an equivalent inline condition) into the actual alert component/markup in that section.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…appear without their month (Sourcery #230)
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
1 participant
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.
Quick wins — alerte nominative + formulaire compact (retours @Thierry 2026-07-19)
1. L'alerte oubli nomme enfin les factures
Avant : « 1 facture de Juin n'a jamais été cochée » (laquelle ?!). Maintenant :
(accords singulier/pluriel corrects, ×5 locales)
2. Formulaire « Ajouter une charge » compact
Fini les champs pleine largeur empilés démesurés : Libellé + Montant partagent une ligne (≥md), et les 3 sélecteurs du CadenceField (Fréquence / Jour / À partir de) tiennent sur une seule ligne. Mobile garde l'empilement vertical.
Scope
UI-only + 1 fonction domaine générique (
unpaidChargesForPeriod— le count en dérive). 76 tests verts sur les 4 suites touchées.Note : ceci ne répond qu'aux 2 points immédiats de tes retours. L'historique par mois, le modèle dettes/échéanciers (SPF, crédits) et la passe UI globale = chantiers cadrés séparément (cf. discussion).
🤖 Generated with Claude Code
Summary by Sourcery
Nommer les factures précédemment oubliées dans l’alerte du tableau de bord et compacter la mise en page du formulaire d’ajout de charge.
Améliorations :
unpaidChargesForPeriodpour retourner les charges impayées pour un mois tout en conservant des champs plus riches côté appelant.ProchainesFacturesCardpour accepter des libellés de factures oubliées plutôt qu’un compteur et rendre un texte d’alerte localisé qui nomme les factures impayées.CadenceFieldet du formulaire d’ajout de charge afin que les champs liés partagent une seule ligne, tout en conservant la pile verticale sur mobile.Tests :
ProchainesFacturesCardpour couvrir le nouveau helper et l’alerte des factures oubliées libellées, y compris les messages au singulier et au pluriel.Original summary in English
Summary by Sourcery
Name previously forgotten bills in the dashboard alert and compact the add-charge form layout.
Enhancements:
Tests: