Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Amélioration de l'affichage des métrics #1277

Draft
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

Maxine-Le-Pennec
Copy link
Contributor

@Maxine-Le-Pennec Maxine-Le-Pennec commented Aug 24, 2022

Dans le cadre de l'amélioration de l'affichage des données métriques grâce à des composants réutilisables, deux éléments ont été modifiés ou créés.

  • Des cartes indiquant des données numériques
    Déjà utilisées dans la page commune, mais ayant besoin d'être adaptée pour s'afficher également sur l'explorateur.

Capture d’écran 2022-08-29 à 10 43 45

  • Une barre de progression
    Cette barre servira dans un premier temps à indiquer le niveau d'avancement des certifications d'adresses sur l'explorateur.
    Elle est accompagnée d'informations telles que le nombre d'adresses précises étant certifiées/non-certifiées.

Capture d’écran 2022-08-29 à 10 42 35

Capture d’écran 2022-08-29 à 10 42 58

@Maxine-Le-Pennec Maxine-Le-Pennec self-assigned this Aug 24, 2022
@jdesboeufs jdesboeufs temporarily deployed to adressedgv-maxine-metri-vttgdt August 24, 2022 15:19 Inactive
@Maxine-Le-Pennec Maxine-Le-Pennec marked this pull request as ready for review August 25, 2022 07:33
Copy link
Contributor

@GllmR GllmR left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il y a un petit souci avec les couleurs apparemment :

image

@tmerlier
Copy link
Contributor

Les métriques sous la barre de progression ne semblent pas très pertinentes dans le cas où toutes les adresses sont certifiées ou non.

Nous pourrions juste afficher x adresses non certifiées pour 0% de certification ou x adresses certifiées pour 100%

@tmerlier
Copy link
Contributor

L'affichage mobile subit un peu le manque de contrainte.
Capture d’écran 2022-08-25 à 14 10 15

components/number-card.js Outdated Show resolved Hide resolved
components/number-card.js Outdated Show resolved Hide resolved
components/number-card.js Outdated Show resolved Hide resolved
components/base-adresse-nationale/commune/details.js Outdated Show resolved Hide resolved
@@ -30,46 +26,28 @@ function Commune({nomCommune, codeCommune, region, departement, voies, nbVoies,
<>
<div className='heading'>
<div className='name-certification'>
<Image src='/images/icons/commune.svg' height={50} width={50} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je pense que nous allons devoir prévoir de quoi adapter la taille de l'image, au moins pour le mobile.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je pensais utiliser flex-wrap mais je ne vois pas comment centrer l'image au moment du wrap sans utiliser de media-q

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrigé via le commit 7e5faf2

@jdesboeufs jdesboeufs temporarily deployed to adressedgv-maxine-metri-vttgdt August 25, 2022 15:25 Inactive
@Maxine-Le-Pennec
Copy link
Contributor Author

Les métriques sous la barre de progression ne semblent pas très pertinentes dans le cas où toutes les adresses sont certifiées ou non.

Nous pourrions juste afficher x adresses non certifiées pour 0% de certification ou x adresses certifiées pour 100%

Je pense que ça va être un peu redondant avec l'icône de certification survolable à côté de la barre de progression. Autant ne rien mettre non ?

J'ai testé et je trouve que ça fait un peu trop de répétition :
Capture d’écran 2022-08-26 à 10 10 08

@tmerlier
Copy link
Contributor

Non, car on a le nombre d'adresses de la commune.

@jdesboeufs jdesboeufs had a problem deploying to adressedgv-maxine-metri-vttgdt August 30, 2022 15:18 Failure
@Maxine-Le-Pennec
Copy link
Contributor Author

Non, car on a le nombre d'adresses de la commune.

Résolu via le commit e7bec1b

@jdesboeufs jdesboeufs temporarily deployed to adressedgv-maxine-metri-vttgdt August 31, 2022 09:45 Inactive
@tmerlier
Copy link
Contributor

On manque un peu de padding
Capture d’écran 2022-08-31 à 11 51 44

@jdesboeufs jdesboeufs temporarily deployed to adressedgv-maxine-metri-vttgdt August 31, 2022 13:17 Inactive
@Maxine-Le-Pennec
Copy link
Contributor Author

On manque un peu de padding Capture d’écran 2022-08-31 à 11 51 44

Résolu via le commit #1277 (comment)

@mmortier
Copy link
Contributor

Je ne vois pas beaucoup de différence avec l'original :
PR
A gauche le résultat en local sur cette branche et à droite l'original.
Ets-ce que ça vaut le coup? Je trouve qu'on comprend moins bien le terme "répertorié"

@tmerlier
Copy link
Contributor

Je ne vois pas beaucoup de différence avec l'original :
PR
A gauche le résultat en local sur cette branche et à droite l'original.
Ets-ce que ça vaut le coup? Je trouve qu'on comprend moins bien le terme "répertorié"

Il ne s'agit là que d'une partie des modifications. @Maxine-Le-Pennec a décrit l'intégralité des changements en description de la PR. La majeure partie des évolutions graphiques se trouve surtout sur la carte BAN.

Attention d'ailleurs à ne pas seulement regarder le rendu, mais aussi le code. Cette PR propose de partager des composants entre ces différentes pages.

@mmortier
Copy link
Contributor

Ok là sur la carte BAN il y a du changement. Mais comme il y a eu des modifications qui ont été au final supprimées c'était un peu dur à suivre..

@mmortier mmortier self-requested a review December 14, 2022 13:49
<div>Nombre de lieux-dits</div>
<div>{nbLieuxDits}</div>
</div>
<Counter label={nbVoies <= 1 ? 'Voie répertoriée' : 'Voies répertoriées'} value={nbVoies} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je laisserai "Voie" et "Voies" pour être raccord avec les termes qu'on utilise dans la carte BAN. J'enlèverai le terme "répertoriée(s)" que je ne trouve pas clair.

<div>{nbLieuxDits}</div>
</div>
<Counter label={nbVoies <= 1 ? 'Voie répertoriée' : 'Voies répertoriées'} value={nbVoies} />
<Counter label={nbLieuxDits <= 1 ? 'Lieu-dit répertorié' : 'Lieux-dits répertoriés'} value={nbLieuxDits} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem ci dessus enlever "répertorié"

</div>
<Counter label={nbVoies <= 1 ? 'Voie répertoriée' : 'Voies répertoriées'} value={nbVoies} />
<Counter label={nbLieuxDits <= 1 ? 'Lieu-dit répertorié' : 'Lieux-dits répertoriés'} value={nbLieuxDits} />
<Counter label={nbNumeros <= 1 ? 'Numéro répertorié' : 'Numéros répertoriés'} value={nbNumeros} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem

@Maxine-Le-Pennec Maxine-Le-Pennec removed their assignment Dec 14, 2022
@GllmR GllmR removed their request for review December 14, 2022 14:36
@nkokla nkokla marked this pull request as draft January 26, 2023 10:55
@tmerlier tmerlier removed their request for review November 6, 2023 08:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Bloqué
Development

Successfully merging this pull request may close these issues.

5 participants