fix(ui): la barre de progression des engagements peignait toujours 100 % - #297
fix(ui): la barre de progression des engagements peignait toujours 100 %#297thierryvm wants to merge 1 commit into
Conversation
Found while verifying #296. Measured in Chromium under the real CSP, on a plan at 4 out of 11 instalments: aria-valuenow="36", the fill carried style="width:36%", and its computed width was 1054px on a 1054px track. Every plan read as fully repaid. `style-src` is `self` + nonce (src/proxy.ts:60) — with no `unsafe-inline` at all in production. A nonce voids `unsafe-inline`, and with no `style-src-attr` the policy governs style ATTRIBUTES too, so the fill width was dropped at parse time and the div kept its natural full width. Screen readers read the right number off aria-valuenow throughout, so the defect was invisible to anything but a rendered page. In an app about money, that bar told @Thierry his payment plan was finished. The comment above it claimed the opposite: "CSP-safe: attribute, not an inline <style>". Three other components (progress.tsx, AllocationBar, Sheet) carry comments getting it right and citing THI-322 — this was the one place that hand-rolled a bar instead of using the shared primitive. - CommitmentsClient uses `<Progress>`, whose fill is an SVG rect with an attribute geometry. Explicit `tone="brand"`: the primitive's auto-tone turns warning past 85 %, correct for a budget being consumed and wrong for a debt being repaid. - `Progress` gains `ariaLabel` (accessible name without a visible caption) and `testId` (on the progressbar element, where the existing assertions look). - The regression tests assert the ABSENCE of any inline style attribute and the rect geometry. The four pre-existing assertions all read aria-valuenow and stayed green throughout the defect, so asserting the value again would not have caught it. Measured after the fix, same harness: painted 0 % / 36 % / 100 % against aria-valuenow 0 / 36 / 100, zero inline styles inside the bars, and zero application-owned CSP violations left (the remaining ones on any page are `style-src-elem` from the Next.js dev overlay). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Guide du relecteurRéorganise la barre de progression des engagements pour utiliser le composant partagé de type SVG Modifications par fichier
Conseils et commandesInteragir avec Sourcery
Personnaliser votre expérienceAccédez à votre tableau de bord pour :
Obtenir de l’aide
Original review guide in EnglishReviewer's GuideRefactors the commitments progress bar to use the shared SVG-based Progress primitive instead of a width set via inline styles, adds accessibility/testing props to Progress, and strengthens tests to ensure CSP-safe rendering and correct geometry of the bar under different values. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Salut – j’ai laissé quelques commentaires généraux :
- Les commentaires en ligne expliquant le comportement de la CSP et le défaut précédent sont assez longs et détaillés ; envisage de les raccourcir et de faire plutôt référence à un ticket ou un document de conception afin de garder la base de code plus facile à parcourir.
- Le JSDoc pour
ProgressProps.labelindique toujours qu’il est aussi utilisé comme nom accessible, mais commeariaLabela désormais la priorité, cette description devrait être mise à jour pour refléter correctement la logique de nommage.
Invite pour les agents IA
Merci de traiter les commentaires de cette revue de code :
## Commentaires généraux
- Les commentaires en ligne expliquant le comportement de la CSP et le défaut précédent sont assez longs et détaillés ; envisage de les raccourcir et de faire plutôt référence à un ticket ou un document de conception afin de garder la base de code plus facile à parcourir.
- Le JSDoc pour `ProgressProps.label` indique toujours qu’il est aussi utilisé comme nom accessible, mais comme `ariaLabel` a désormais la priorité, cette description devrait être mise à jour pour refléter correctement la logique de nommage.Sourcery est gratuit pour l’open source – si tu apprécies nos revues, merci d’en parler ✨
Original comment in English
Hey - I've left some high level feedback:
- The inline comments explaining the CSP behavior and prior defect are quite long and detailed; consider shortening them and instead referencing a ticket or design doc to keep the codebase easier to scan.
- The JSDoc for
ProgressProps.labelstill says it is also used as the accessible name, but withariaLabelnow taking precedence this description should be updated to accurately reflect the naming logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The inline comments explaining the CSP behavior and prior defect are quite long and detailed; consider shortening them and instead referencing a ticket or design doc to keep the codebase easier to scan.
- The JSDoc for `ProgressProps.label` still says it is also used as the accessible name, but with `ariaLabel` now taking precedence this description should be updated to accurately reflect the naming logic.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Le défaut
La barre de progression de
/app/commitmentspeignait 100 % quelle que soit la valeur. Sur un plan à 4 échéances sur 11, elle disait « soldé ».Ce n'est pas cosmétique : c'est un chiffre faux dans une application de finances. L'utilisateur regarde cette barre pour savoir où il en est de son plan de paiement.
Mesuré, pas déduit
Chromium,
npm run dev, stack Supabase locale, plan à 4/11 :aria-valuenow36✅styledu remplissagewidth:36%✅ présentsrc/proxy.ts:60:style-src: 'self' 'nonce-…'— et en production, sans'unsafe-inline'du tout (l'extra n'est ajouté qu'en dev, où le navigateur l'ignore de toute façon puisqu'un nonce est présent). Un nonce annule'unsafe-inline', et faute destyle-src-attrla politique gouverne aussi les attributsstyle. La largeur était donc jetée à l'analyse du document, et le div gardait sa largeur naturelle : 100 %.Le lecteur d'écran recevait la bonne valeur via
aria-valuenowpendant tout ce temps. Le défaut n'était visible que sur une page rendue — d'où sa survie.Et le commentaire au-dessus affirmait l'inverse : « CSP-safe: attribute, not an inline
<style>». Trois autres composants (ui/progress.tsx,AllocationBar.tsx,Sheet.tsx) portent des commentaires qui disent juste et citent THI-322. C'était le seul endroit à réimplémenter une barre à la main au lieu d'utiliser la primitive partagée.Les autres composants — vérifiés, pas supposés
Balayage de tous les
style={{du dépôt, puis audit DOM de chaque élément portant un attributstylesur/,/login,/signup,/en:src/app/[locale]/opengraph-image.tsx—next/og(satori), génère un PNG côté serveur. Aucune CSP, non concerné.styleobservés en page est posé par JavaScript via le CSSOM (--consent-height, portail des devtools Next, route announcer) — que la CSP n'intercepte pas.Après correctif, les violations restantes sont toutes
style-src-elemen provenance denext-devtools— outillage de développement, absent du build de production. Zéro violation imputable à l'application.Le correctif
CommitmentsClientconsomme<Progress>, dont le remplissage est un<rect>SVG à géométrie attributaire — c'est précisément la raison d'être de cette primitive (THI-322).tone="brand"explicite : l'auto-tone de la primitive vire àwarningau-delà de 85 %, ce qui est juste pour un budget qu'on consomme et faux pour une dette qu'on rembourse, où frôler 100 % est la bonne nouvelle.Progressgagne deux props :ariaLabel— nom accessible sans légende visible (labelen affiche une). La ligne nomme déjà l'engagement à l'écran ; la répéter serait du bruit, mais une barre anonyme laisserait un utilisateur d'AT sans savoir à quelle ligne elle appartient.testId— posé sur l'élémentrole="progressbar", là où les assertions existantes regardent.Les tests portent sur le mécanisme, pas sur la valeur
Les quatre assertions préexistantes lisaient
aria-valuenow— elles sont restées vertes pendant tout le défaut. Réaffirmer la valeur n'aurait rien attrapé. Les nouveaux tests assertent donc ce que la politique tue :stylesous la barre (querySelectorAll('[style]')vide) ;<rect>suit la valeur (12 à 2/17, 0 à 0 payé) ;ariaLabelsans légende visible, priorité surlabel,testIdsur le bon élément, et « n'exprime jamais sa géométrie par un attributstyle».Vérification
lint0 erreur ·typecheck0 ·test140 fichiers / 1847 cas, 100 % ·build✅ ·npm run devdémarre et sert (.nextpurgé entre les deux).Mesuré dans le navigateur après correctif, trois plans côte à côte :
aria-valuenowSignalé, non corrigé — avertissement d'hydratation
Présent sur
/et/loginavant cette PR. La trace le nomme précisément :Le serveur rend le
<script>de bascule de thème avec le nonce, le client le reconstruit avecnonce=""— React effacenoncedes propriétés DOM après hydratation, par conception (défense contre l'exfiltration de nonce). Le correctif habituel est unsuppressHydrationWarningsur ce<script>précis, ou la lecture du nonce côté client viaprops.nonceplutôt que l'attribut. Hors périmètre ici : CLAUDE.md exige zéro avertissement console en dev, donc ça mérite sa propre PR plutôt qu'un passager clandestin dans celle-ci.🤖 Generated with Claude Code
Résumé par Sourcery
Remplacer la barre de progression des engagements par le composant partagé Progress UI afin d’afficher une complétion visuelle fidèle dans le cadre du CSP strict de l’application.
Nouvelles fonctionnalités :
ariaLabelettestIdpour une meilleure accessibilité et des tests plus précis.Corrections de bugs :
Améliorations :
CommitmentsClientqui vérifient que la barre de progression rendue utilise la géométrie SVG sans attributs de style inline et que la largeur suit correctement le ratio payé.Original summary in English
Summary by Sourcery
Replace the commitments progress bar with the shared Progress UI primitive to render accurate visual completion under the app’s strict CSP.
New Features:
Bug Fixes:
Enhancements: