betagouv / reno

Mes Aides Réno : Aides à la rénovation 2024 + coût des travaux
https://mesaidesreno.beta.gouv.fr
MIT License
14 stars 4 forks source link

API version 2 #181

Closed laem closed 2 months ago

laem commented 3 months ago
vercel[bot] commented 3 months ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
reno ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 9, 2024 8:48am
reno-dev ❌ Failed (Inspect) Sep 9, 2024 8:48am
morganmerzouk commented 2 months ago

@laem Je veux bien une review, je ne compte pas forcément aller beaucoup plus loin dans un 1er temps sauf si tu me dis qu'il manque des choses importantes

laem commented 2 months ago

@morganmerzouk une petite remarque sur le HTML : mieux vaut privilégier les balises html existantes, comme le <details qui fait en natif ce que tu fais ici via le composant <Tabs. Pas rédhibitoire pour cette PR, mais à noter pour la suite.

laem commented 2 months ago

@morganmerzouk beau boulot ! La page est 10 x mieux. Juste un point qui me semble important avant de merger : ça te semble faisable rapidement de proposer une option dans les paramètres d'entrée qui ferait que l'API renvoie l'object d'évaluation complet ?

La version actuelle, la valeur formatée, est bien mais me semble quand même limitée pour d'autres usages. En pratique, dans notre code, on a souvent besoin de l'objet, la valeur formatée ne permettant pas, par exemple, de séparer l'affichage de la valeur numérique de l'unité (genre, leur donner une couleur différente) sans utiliser des regexp. Ou encore, l'arbre de calcul.

Ce serait donc une option d'alourdissement du payload pour ceux qui en veulent.

morganmerzouk commented 2 months ago

@morganmerzouk beau boulot ! La page est 10 x mieux. Juste un point qui me semble important avant de merger : ça te semble faisable rapidement de proposer une option dans les paramètres d'entrée qui ferait que l'API renvoie l'object d'évaluation complet ?

La version actuelle, la valeur formatée, est bien mais me semble quand même limitée pour d'autres usages. En pratique, dans notre code, on a souvent besoin de l'objet, la valeur formatée ne permettant pas, par exemple, de séparer l'affichage de la valeur numérique de l'unité (genre, leur donner une couleur différente) sans utiliser des regexp. Ou encore, l'arbre de calcul.

Ce serait donc une option d'alourdissement du payload pour ceux qui en veulent.

Ok, je vais:

laem commented 2 months ago

Super ! N'hésite pas à merger quand c'est prêt. Idéalement on fait une démo aux banques tout à l'heure.