refactor: unify budget grid card and add list row by neogenz · Pull Request #315 · neogenz/pulpe

neogenz · 2026-02-08T17:15:47Z

Refactor budget grid component by:

Consolidating mobile and desktop card into unified component
Adding new list row view for alternative layout
Removing dedicated mobile card component
Updating grid exports

vercel · 2026-02-08T17:15:52Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
pulpe-frontend	Ready	Preview, Comment	Feb 8, 2026 5:17pm

supabase · 2026-02-08T17:15:54Z

This pull request has been ignored for the connected project xvrbcvltpkqwiiexvfxh because there are no changes detected in backend-nest/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.

Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

claude · 2026-02-08T17:17:49Z

🔍 Review PR #315

Résumé: 🚨 1 bloquant · ⚠️ 2 importants · 💡 2 suggestions
Risque: 🟡 Moyen · Fichiers: 6

🚨 Bloquants (merge impossible)

budget-grid-card.spec.ts:10-42 — Tests qui testent l'implémentation, pas le comportement

Les tests "Card Click Logic" testent une fonction isolée au lieu du comportement réel du composant. Ceci est une violation critique du principe "tester le comportement, pas l'implémentation".

Impact: Ces tests ne valident pas que le composant fonctionne correctement. Si la logique interne change (par ex. renommer onCardClick), les tests cassent même si le comportement reste correct.

Fix: Soit supprimer ces tests (car E2E couvre ce cas), soit utiliser un vrai test de composant avec TestBed si le workaround input.required() peut être contourné.

⚠️ Importants

budget-grid.ts:247-249 — Utilisation de ::ng-deep interdit

L'utilisation de ::ng-deep pour styler .mat-expansion-panel-body est explicitement interdite par les best practices Angular du projet. Ce sélecteur est déprécié et brise l'encapsulation des styles.

Solution: Utiliser ViewEncapsulation.None sur le composant ou appliquer le style via une classe globale dans styles.scss. Exemple:
// Dans budget-grid.ts
@Component({
  // ...
  encapsulation: ViewEncapsulation.None,
})

budget-list-row.ts:148 — Output viewTransactions déclaré mais jamais émis

L'output viewTransactions est déclaré dans le composant mais n'est jamais utilisé. Le composant n'a pas de bouton pour "voir les transactions" contrairement à budget-grid-card.ts:167-184.

Impact: Dead code qui crée de la confusion. Les développeurs pourraient essayer de se connecter à cet output qui ne sera jamais déclenché.

Fix: Soit supprimer l'output s'il n'est pas nécessaire dans la vue "list", soit ajouter un bouton équivalent au card.

💡 Suggestions

budget-grid-card.spec.ts:39-71 — Tests "Category Grouping" mal placés

Les tests de groupement par catégorie (lignes 39-71) sont dans budget-grid-card.spec.ts mais testent une logique qui appartient à budget-grid.ts (computed categories). Ce n'est pas le bon fichier.

Suggestion: Déplacer ces tests vers un fichier budget-grid.spec.ts ou les supprimer si couverts par E2E.

budget-grid.ts:282-283 — Fonction inline dans computed pourrait être extraite

La fonction calcTotal est définie inline dans le computed. Pour améliorer la lisibilité et permettre les tests unitaires, considérer l'extraction comme méthode privée.
#calcTotal(list: BudgetLineTableItem[]): number {
  return list.reduce((sum, i) => sum + i.data.amount, 0);
}

✨ Points positifs

✅ Excellente unification mobile/desktop — réduction de duplication de code majeure
✅ OnPush changeDetection respecté sur tous les composants
✅ Utilisation correcte de input() / output() (pas @Input/@output)
✅ Accessibilité ARIA bien gérée (aria-label sur toggles)
✅ Bonne gestion du stopPropagation pour éviter les clicks bubbling
✅ Design tokens Material 3 correctement appliqués
✅ Pattern AAA respecté dans les tests qui le font (Arrange/Act/Assert)
✅ data-testid présents pour les tests E2E

Verdict: ❌ CHANGES REQUESTED

Corriger le ::ng-deep bloquant et l'output dead code, puis re-request review.

claude · 2026-02-08T17:17:57Z

frontend/projects/webapp/src/app/feature/budget/budget-details/budget-grid/budget-grid.ts

+
+    ::ng-deep .mat-expansion-panel-body {
+      padding: 0 !important;
+    }


🚨 Bloquant: Utilisation de ::ng-deep interdite

L'utilisation de ::ng-deep est explicitement interdite par les best practices Angular du projet. Ce sélecteur est déprécié et brise l'encapsulation des styles.

Solution recommandée:

// Option 1: ViewEncapsulation.None import { ViewEncapsulation } from '@angular/core'; @Component({ // ... encapsulation: ViewEncapsulation.None, })

Ou ajouter dans un fichier de styles global (styles.scss).

claude · 2026-02-08T17:17:59Z

frontend/projects/webapp/src/app/feature/budget/budget-details/budget-grid/budget-list-row.ts

+  readonly edit = output<BudgetLineTableItem>();
+  readonly delete = output<string>();
+  readonly addTransaction = output<BudgetLine>();
+  readonly viewTransactions = output<BudgetLineTableItem>();


⚠️ Important: Output viewTransactions déclaré mais jamais émis

Cet output est déclaré mais aucun bouton dans le template ne l'émet. Contrairement à BudgetGridCard (lignes 167-184), BudgetListRow n'a pas de bouton pour voir les transactions.

Impact: Dead code qui crée de la confusion.

Fix: Supprimer l'output ou ajouter un bouton équivalent au card.

claude · 2026-02-08T17:18:02Z

...d/projects/webapp/src/app/feature/budget/budget-details/budget-grid/budget-grid-card.spec.ts

+  describe('Card Click Logic', () => {
+    it('should not emit cardClick when isMobile is true', () => {


🚨 Bloquant: Tests qui testent l'implémentation, pas le comportement

Ces tests violent le principe critique "tester le comportement, pas l'implémentation". Ils testent une fonction isolée au lieu du comportement réel du composant.

Problème: Si vous renommez onCardClick ou changez la logique interne, ces tests cassent même si le comportement utilisateur reste correct.

Fix:

Soit supprimer ces tests (E2E couvre déjà ce cas)

Soit utiliser un vrai test de composant avec TestBed si le workaround input.required() peut être contourné

claude · 2026-02-08T17:18:04Z

...d/projects/webapp/src/app/feature/budget/budget-details/budget-grid/budget-grid-card.spec.ts

+    });
+  });
+
+  describe('Category Grouping', () => {


💡 Suggestion: Tests mal placés

Ces tests "Category Grouping" sont dans budget-grid-card.spec.ts mais testent la logique du computed categories qui appartient à budget-grid.ts.

Suggestion: Déplacer vers budget-grid.spec.ts ou supprimer si couverts par E2E.

claude · 2026-02-08T17:18:05Z

frontend/projects/webapp/src/app/feature/budget/budget-details/budget-grid/budget-grid.ts

-    };
-    return [income, saving, expense];
+
+    const calcTotal = (list: BudgetLineTableItem[]) =>


💡 Suggestion: Extraire la fonction inline

La fonction calcTotal pourrait être extraite comme méthode privée pour améliorer la lisibilité et permettre les tests unitaires:

#calcTotal(list: BudgetLineTableItem[]): number { return list.reduce((sum, i) => sum + i.data.amount, 0); }

github-actions · 2026-02-08T17:25:12Z

🎭 Playwright E2E Tests

74 tests ±0 73 ✅ - 1 4m 17s ⏱️ +16s
17 suites ±0 0 💤 ±0
1 files ±0 1 ❌ +1

For more details on these failures, see this check.

Results for commit e4f4453. ± Comparison against base commit 08a4bb1.

♻️ This comment has been updated with latest results.

refactor(budget-grid): unify card component and add list row view

e4f4453

vercel bot deployed to Preview February 8, 2026 17:17 View deployment

claude bot reviewed Feb 8, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: unify budget grid card and add list row#315

refactor: unify budget grid card and add list row#315
neogenz wants to merge 1 commit intomainfrom
neogenz/budget-grid-list-row

neogenz commented Feb 8, 2026

Uh oh!

vercel bot commented Feb 8, 2026 •

edited

Loading

Uh oh!

supabase bot commented Feb 8, 2026

Uh oh!

claude bot commented Feb 8, 2026

Uh oh!

claude bot Feb 8, 2026

Uh oh!

claude bot Feb 8, 2026

Uh oh!

claude bot Feb 8, 2026

Uh oh!

claude bot Feb 8, 2026

Uh oh!

claude bot Feb 8, 2026

Uh oh!

github-actions bot commented Feb 8, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		describe('Card Click Logic', () => {
		it('should not emit cardClick when isMobile is true', () => {

Conversation

neogenz commented Feb 8, 2026

Uh oh!

vercel bot commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

supabase bot commented Feb 8, 2026

Uh oh!

claude bot commented Feb 8, 2026

🔍 Review PR #315

🚨 Bloquants (merge impossible)

⚠️ Importants

💡 Suggestions

✨ Points positifs

Uh oh!

claude bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

claude bot Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎭 Playwright E2E Tests

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel bot commented Feb 8, 2026 •

edited

Loading

github-actions bot commented Feb 8, 2026 •

edited

Loading