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

Feature/result page #49

Merged
merged 29 commits into from
Feb 4, 2022
Merged

Feature/result page #49

merged 29 commits into from
Feb 4, 2022

Conversation

Bastou
Copy link
Contributor

@Bastou Bastou commented Jan 19, 2022

Une pr conséquente pour la page résultat (j'ai eu du mal à splitter). L'interface est intégré et le contenu est réactif en fonction des paramètres d'urls passés à la page pour récupérer les résultats de l'api.
Deux méthodes possibles :

  • Par l'id : paramètre d'url ?id=myPageId va requêter l'api pour mettre à jour le contenu du dom
  • Par les infos de résultat directement dans l'url, exemple : ?width=1920&height=1080&url=https%3A%2F%2Fwww.leroymerlin.fr&grade=E&score=34&ges=2.32&water=3.48&date=2021-11-17T12%3A40%3A18.575464&page_type=null&id=2d43d4c9-6ad0-4dc8-a769-09b3b2249bf3&version=1&size=1119.963&nodes=1286&requests=65&host=www.leroymerlin.fr

Que contient cette pr :

  • Intégration du front de la page résultat (desktop/mobile)
  • Lien entre l'api et le contenu de la page résultat mis à jour en fonction des infos d'analyse
  • Ajout des variables de couleurs pour les grades de résultats
  • Ajout d'un composant "Collapse" pour les infos complémentaires des sections de la page résultat
  • Ajout d'un composant "Slider range" pour les données de poids, nombres de requetes et complexité
  • Ajouts d'icônes dans les symboles pour la page résultat
  • Ajout d'un composant et shortcode pour les liens soulignés dans le contenu
  • Ajout d'un paramètre pour ajouter des classes custom à des widgets
  • Ajout d'un paramètre pour ajouter des classes custom à des contenus de page

Ce qu'il reste à faire :

  • Contenu éditable en markdown (paramètres frontmatter)
  • Partie en de la page /result à mettre à jour avec le markdown
  • Corrections sur mobile sur des calages d'éléments
  • Carrés de couleurs correspondant aux résultats de page dans appreciations

@yaaax j'ai ajouté des classes d'helpers dans base-structure je peux te faire une mr sur gitlab ?

⚠️ Pr basé sur feature/split-css, sera à merger avant

Aperçu :

@Bastou Bastou added the enhancement New feature or request label Jan 19, 2022
@Bastou Bastou requested a review from yaaax January 19, 2022 21:28
@Bastou Bastou self-assigned this Jan 19, 2022
@Bastou Bastou changed the title Feature/result page Draft: Feature/result page Jan 19, 2022
@yaacov
Copy link

yaacov commented Jan 19, 2022

@yaacov j'ai ajouté des classes d'helpers dans base-structure je peux te faire une mr sur gitlab ?

Thank you, but I think I'm not the yaacov you are looking for.

@vvatelot
Copy link
Member

Hello @Bastou, l'url de l'api n'est pas censee etre publique. Il faut passer par l'url rapidapi

@yaaax
Copy link
Contributor

yaaax commented Jan 21, 2022

Super boulot !

Je note pour les widgets en draft 👍 (attention il faudrait modifier le commentaire "get widget custom class" copié-collé du dessus dans widgets.html.
Je note aussi pour le nouveau paramètre de widget widget_class 👍

Pour les helpers, de quoi s'agit-il exactement ? Quand je teste cette PR en local je vois que le collapse ne fonctionne pas. J'imagine qu'il y aurait au moins la classe .visually-hidden ? Elle existe, c'est .vh ;)
Sinon je ne vois pas les classes collapse-content et js-collapse-content dans le CSS, est-ce qu'elles servent vraiment ?

Contenu

Je vois 2 informations contradictoires : "une note de A à G" à gauche, et l'échelle de note (très jolie !) qui ne va que jusqu'à F. Sur la maquette InVision je vois une lettre G en rouge…
@aureliebaton A à F ou A à G ?

Fonctionnel

Le lien "Voir les détails du score" doit renvoyer plus bas dans la page (ancre) au titre <h2> "Détail du score".
Je vais m'occuper d'ajouter des ids à tous les titres !

Accessibilité

Avec ma casquette actuelle d'auditeur accessibilité, forcément je vois des choses qui ne passent pas :)

Élement Collapse

  • le focus est invisible sur les boutons de collapse (pas de gros outline vert comme sur les autres éléments focusables)
  • les liens dans un panneau collapsé sont toujours focusables alors qu'ils ne devraient pas l'être (tester en appuyant sur Tab). Par ex. le lien vers "Comment ça marche"

L'exemple à suivre est le motif de conception Disclosure, et l'exemple que tu peux regarder : "Example Disclosure (Show/Hide) for Answers to Frequently Asked Questions"

Mais tu sais quoi, maintenant que j'y pense, ne te prends pas trop la tête avec ça, j'avais commencé un module hugo proposant quelques composants d'interface accessibles, et j'avais justement commencé par le "Collapsible Section" du site Inclusive Components. Je devrais pouvoir l'intégrer (une fois cette PR mergée)

Hiérarchie des titres

  • Attention il y a des <h2> sans <h1> ("Résultat pour www.leroymerlin.fr" devrait certainement être un <h1>
  • les <h4> devraient être des <h3> (car ils sont sous des <h2>)

Contrastes

  • Le bleu foncé sur vert n'est pas assez contrasté (par ex. "Si légère", "1.12 Mo", "Peu de requêtes", etc.

Je te laisse corriger ce que tu peux avant de merger ? Ou on merge et on corrige après ?

@Bastou
Copy link
Contributor Author

Bastou commented Jan 25, 2022

Merci pour ton retour @yaaax. Je vais corriger déjà ces éléments sur cette pr. On pourra merger après.

@yaaax yaaax marked this pull request as ready for review February 4, 2022 10:16
@yaaax
Copy link
Contributor

yaaax commented Feb 4, 2022

Merci @Bastou pour les corrections :

  • note de A à G
  • focus sur l'élément Collapse / classe .vh
  • hiérarchie des titres

⚠️ Attention il reste toujours le problème de contraste

Le bleu foncé sur vert n'est pas assez contrasté (par ex. "Si légère", "1.12 Mo", "Peu de requêtes", etc.

J'ai créé une issue pour @Bastou :

Il me reste les tâches de mon côté :

Je merge donc ce PR

@yaaax yaaax closed this Feb 4, 2022
Copy link
Contributor

@yaaax yaaax left a comment

Choose a reason for hiding this comment

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

OK

@yaaax yaaax reopened this Feb 4, 2022
@yaaax yaaax merged commit b679c07 into master Feb 4, 2022
@yaaax yaaax deleted the feature/result-page branch February 4, 2022 10:30
@Bastou Bastou changed the title Draft: Feature/result page Feature/result page Feb 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants