Ajout de la fonctionnalité permettant de choisir le type de vaccin #143
Conversation
…au départ et que le typeVaccin ait une valeur par défaut
…e par departement. Corrections de bug. Merci à nouveau @hawksttf
src/routing/Router.ts
Outdated
| @@ -50,22 +50,23 @@ class Routing { | |||
| `/centres-vaccination-covid-dpt:codeDpt-:nomDpt/age-:trancheAge/`, | |||
| `/centres-vaccination-covid-dpt:codeDpt-:nomDpt/ville-:codeVille-:nomVille/age-:trancheAge/`, | |||
| // Proper URL really used | |||
| `/centres-vaccination-covid-dpt:codeDpt-:nomDpt` | |||
| `/centres-vaccination-covid-dpt:codeDpt-:nomDpt/type-vaccin-:typeVaccin`, | |||
There was a problem hiding this comment.
Pas fan de l'URL. Je pense que juste ARNm ou Adenovaccin serait plus clair
src/views/vmd-rdv.view.ts
Outdated
| <div class="col"> | ||
| <vmd-button-switch class="mb-3" | ||
| codeSelectionne="${this.typeVaccin}" | ||
| .options="${Array.from(FILTRE_TYPE_VACCIN.values()).map(tc => ({code: tc.codeTypeVaccin, libelle: tc.libelle }))}" |
There was a problem hiding this comment.
On avait déjà parlé de ARNm et adenovirus avant, ça serait peut être bien de mettre (AZ/Janseen) et (Pfizer/Moderna) pour que ça soit plus clair
There was a problem hiding this comment.
Il y a un sujet sur la taille des libéllés
There was a problem hiding this comment.
Oui on en avait parlé sur le chat. Il faut que je corrige mais cela va aussi dépendre de où va le filtre au final.
|
On peut faire comme ça si vous préférez (j'ai pris en compte les remarques) :
|
…eur du type de vaccin. Relégation des types en title des boutons. Ajout de title sur les boutons par type de recherche proche/vite. Simplification URL
…le backend (Janssen)
|
Personnellement je préfère, mais je vais laisser d'autres qui ont plus d'ownership sur le front commenter, merci 👍 |
|
welcome aboard, @mccob ! comme discuté en "copil" / "weekly" hier soir avec la team ; ok pour tester la pertinence de l'utilisation d'un filtrage par vaccin auprès des users.
|
|
Il faut en revanche s'assurer que l'on tracke bien l'utilisation de cette feature sur le site : j'ai ajouté une proposition de GTM Tag pour cette feature sur notre plan de tracking excel (ligne 23), que JB Le Prado, peut finaliser et nous valider avant que l'on mette en ligne. --> je lui droppe un msg sur le chan VMD Analytics. plan de tracking une fois que c'est validé avec JB, on est good ! |
src/views/vmd-rdv.view.ts
Outdated
| @@ -404,6 +420,40 @@ export abstract class AbstractVmdRdvView extends LitElement { | |||
| throw new Error(`Unsupported tri : ${tri}`); | |||
| } | |||
| } | |||
|
|
|||
| protected filterTypeVaccin(vaccine_type: Array, typeVaccin: CodeTypeVaccin) { | |||
There was a problem hiding this comment.
Le typage de vaccine_type est vraiment Array?
Plus bas, c'est traité comme un string : vaccine_type.split(',')
src/views/vmd-rdv.view.ts
Outdated
| let result = false; | ||
|
|
||
| if(vaccine_type && vaccine_type.length) | ||
| { | ||
| if(vaccine_type.indexOf(',')!==-1) | ||
| { | ||
| let arrayVaccinne = vaccine_type.split(','); | ||
|
|
||
| arrayVaccinne.forEach(function(item){ | ||
| if(TYPES_VACCIN[item.trim()]==typeVaccin) | ||
| { | ||
| result = true; | ||
| } | ||
| }); | ||
| }else{ | ||
| if(TYPES_VACCIN[vaccine_type.trim()]==typeVaccin) | ||
| { | ||
| result = true; | ||
| } | ||
| } | ||
| return result; | ||
| }else{ | ||
| // Not hidding vaccine center of vaccine type is unknown | ||
| result = true; | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
Hello,
Le code n'est pas immutable à cause de cette variable result. Je pense que tu peux t'en passer.
Et faire directement directement un return true ou return false au lieu de mettre à jour la variable.
There was a problem hiding this comment.
Le problème est qu'elle est nécessaire au moins pour la boucle forEach : la valeur finale du result est nécessaire dans ce cadre.
There was a problem hiding this comment.
J'ai l'impression que tout ça peut se simplifier en :
protected filterTypeVaccin(vaccine_type: String, typeVaccin: CodeTypeVaccin) {
if(typeVaccin === 'tous') {
return true;
} else {
if(!vaccine_type || !vaccine_type.length) {
return false;
}
return vaccine_type.split(",")
.map((typeStr) => TYPES_VACCIN[typeStr.trim() as TypeVaccin])
.indexOf(typeVaccin) !== -1;
}
}
non ?
There was a problem hiding this comment.
La méthode Array.prototype.some() peut faire le job aussi
https://developer.mozilla.org/fr/docs/Web/JavaScript/Reference/Global_Objects/Array/some
src/views/vmd-rdv.view.ts
Outdated
| }else{ | ||
| if(TYPES_VACCIN[vaccine_type.trim()]==typeVaccin) | ||
| { | ||
| result = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
| }else{ | |
| if(TYPES_VACCIN[vaccine_type.trim()]==typeVaccin) | |
| { | |
| result = true; | |
| } | |
| } | |
| }else if(TYPES_VACCIN[vaccine_type.trim()]==typeVaccin) { | |
| result = true; | |
| } |
There was a problem hiding this comment.
il ya un problème avec la touche espace ? ^^
|
Super feature, merci beaucoup ! |
|
Ce dont nous avions parlé lors des précédents échanges était d'avoir une zone expandable "Paramètres avancés" dans le bloc du haut, dans lequel se trouverait le choix du vaccin (et, plus tard, d'autres paramètres tels que la distance en mode recherche par commune uniquement) :
Autres remarques :
J'aurais fait l'inverse : si l'utilisateur décide de filtrer et qu'on ne sait pas, on n'affiche pas le centre. |
Co-authored-by: Bilelz <bilelz@gmail.com>
|
Je vais transmettre les modifications que vous avez proposé.
je ne vois pas trop comment on peut gérer un regroupement ARNm d'un côté et à base d'adénovirus de l'autre sans mettre l'information quelque part côté VMD. à moins que les sites vous indiquent le type de vaccin quelque part ? mais en effet je comprends les réticences à gérer de la complexité. Par contre certains délaissent Moderna car ils ne savent pas que c'est un ARNm comme le PFizer. D'où l'intérêt des les regrouper. https://www.lci.fr/sante/covid-19-moderna-moins-demande-pfizer-2182891.html
La maquette proposée est bien car elle permet de "cacher" la complexité. Par contre, on perds l'intérêt de la simplification/regroupement par type mais en effet si on fait cela, on en revient à devoir gérer la complexité.
OK. Je vais proposer cette modification également si personne n'a de chose à redire sur cette remarque. |
|
Attention, je n'ai pas dit que j'étais contre les regroupements. Par exemple, si le back était en mesure de nous fournir ce niveau de regroupement (en gros, qu'on ait un champ |
| @@ -16,10 +16,27 @@ export type TriCentre = { | |||
| libelle: string; | |||
| }; | |||
| export const TRIS_CENTRE: Map<CodeTriCentre, TriCentre> = new Map([ | |||
| ['distance', { codeTriCentre: 'distance', libelle: "Au plus proche" }], | |||
| ['date', { codeTriCentre: 'date', libelle: "Disponible au plus vite" }], | |||
| ['distance', { codeTriCentre: 'distance', libelle: "Au plus proche", title:"Les lieux de vaccination les plus proches" }], | |||
There was a problem hiding this comment.
Pourquoi cette modification de libellés ? Ça me paraît hors sujet de la PR en cours.
There was a problem hiding this comment.
ce n'est pas une modification de libellés, c'est juste que comme j'ai mis des title sur l'autre sélecteur, j'ai ais mis sur celui ci aussi. J'enlève si cela ne vous va pas.
There was a problem hiding this comment.
oh oui my bad je n'avais pas les yeux en face des trous, je croyais que le libelle avait été remplacé par title, my bad il n'y a pas de soucis à laisser tel quel ! :)
src/routing/Router.ts
Outdated
| @@ -50,22 +50,23 @@ class Routing { | |||
| `/centres-vaccination-covid-dpt:codeDpt-:nomDpt/age-:trancheAge/`, | |||
| `/centres-vaccination-covid-dpt:codeDpt-:nomDpt/ville-:codeVille-:nomVille/age-:trancheAge/`, | |||
| // Proper URL really used | |||
| `/centres-vaccination-covid-dpt:codeDpt-:nomDpt` | |||
| `/centres-vaccination-covid-dpt:codeDpt-:nomDpt/vaccin-:typeVaccin`, | |||
There was a problem hiding this comment.
Il faut rester backward compatible sur les URLs (si Google a référencé l'une de nos "vieilles" urls, il faut qu'on sache continuer de la résoudre correctement)
Ici, la plupart des changements sur les URLs sont backward incompatibles.
Et dans tous les cas, je m'interroge sur la pertinence de passer tout ce qui se trouve dans les "paramètres avancés" sous forme de Path parameters ... j'en ferais bien des query parameters plutôt que des path parameters (ça résoud le problème de backward incompat par la même occasion)
There was a problem hiding this comment.
je ne voulais pas forcément le mettre en paramètre URL le type, c'est juste que je suis parti sur une reprise telle quelle des fonctionnalité comme pour l'autre sélecteur. Mais si on ne veut pas de modification d'URL, on peut revenir dessus.
There was a problem hiding this comment.
@fcamblor mais passer en path plutot que paramètres c'est mieux pour la SEO ;)
There was a problem hiding this comment.
D'un point de vue SEO, c'est mieux de faire passer le typeVaccin en query parameters (comme sur un site ecommerce quand on filtre sur une page liste de produits)
There was a problem hiding this comment.
Il faut trouver un juste milieu entre tout mettre en path parameters (et se tirer une balle pour la backward compat à chaque fois qu'on rajoute un critère de tri/recherche) ou ne mettre que ce que permet à Google de "catégoriser" le site.
Je serais d'avis de mettre :
- en PATH parameters : le département + la commune (si commune sélectionnée)
- en QUERY parameters : le tri + les filtres de second niveau (type de vaccin, distance max, etc..)
@francoisBouchet niveau SEO, tu serait ok avec ça ?
… puisse build [2]
… puisse build [3]
… puisse build [5]
|
J'ai transmis des modifications suite à vos remarques et propositions à nouveau. Les modifications font aussi que cela build sans erreur à présent.
|
En pratique on peut juste supporter les anciens types de vaccins en les gardant dans le JSON jusqu'à ce que ça soit changé sur le front. C'est pas un énorme problème, il faudra juste faire attention. Et pas sûr que ça arrive souvent |
|
Personnellement je suis chaud pour MEP en l'état, et voir si c'est utilisé avant d'investir du temps sur le back pour la qualité de la data + potentiellement le vaccine type en plus du vaccine name |
je pense pas qu'il faille mettre en prod sans s'assurer de remettre la comptabilité des URL backward (je suis d'accord avec @fcamblor). edit : ce point est corrigé. Par contre, il y a sans doute une simplification à faire par rapport à ce que j'ai fait sur le Router. J'ai trouvé une solution pour que cela fonctionne, pas certain que cela soit la meilleure. à voir également les analyticsViewName. Par ailleurs @rozierguillaume voulait rendre la fonctionnalité plus discrète. C'est peut être un prérequis également. à voir les idées sur le sujet en terme de présentation. |
|
Comme @Noezor je pense qu'on peut MEP, et récolter les retours des utilisateurs. Puis (si c'est utilisé) rendre ça plus discret dans un second temps. |
|
je pense qu'on pourrait au moins le foutre en dessous de la selection du tri :) |
|
peut-être plus simple que "bouton + div" dédié à tous les filtres, on peut placer le filtrage par vaccin déjà développé en début de listing en desktop (plutôt que dans le bloc d'entête)...
ça allège l'entête mais ça règle pas le problème en smartphone... en smartphone je placerai un simple select en bottom... mais... si on l'affiche par défaut, on a toujours un début de page complètement encombré. il faudrait le faire apparaitre en bottom lorsque l'utilisateur commence à scroller et a passé les 500/600px de scroll...
un bout de code trouvé sur les zinternets : |
|
nb: j'avais pas vu qu'on avait regroupé les vaccins par typologie... le principe reste le même... |
|
@mccob : JBLePrado a posé les specs analytics sur le plan de tagging disponible à cette url, enjoy! >> https://docs.google.com/spreadsheets/d/1dhANE4U_Xlo_U_NpCQC3doP7emkoQ3rW0iQibdwJqB0/edit?usp=sharing |
|
Je suis vraiment pas fan de mettre des filtres dans des blocs différents.. Et en sticky bottom ENCORE moins (espace "systématiquement" utilisé, potentiellement pour rien ... et quand je disais que je voulais rester discret sur ce filtre, tu me mets un truc prépondérant qui disparaît même pas quand on scroll :( ) |
|
ah ben je suis pas fan non plus, mais là je suis en mode "trouver la moins mauvaise des solutions... qui soit plus simple que mon bouton filtre + layer dédié... et qui permette d'alléger l'entête de page" :-/ |
Merci. J'ai l'impression que vous avez repris ce que j'avais défini au final. Du coup je n'ai rien à changer ? à part que je vois dans le document :
Est-il possible d'en savoir plus sur l'implémentation à réaliser ? Sinon je retiens que pour le moment il n'est pas tranché où doit apparaitre le choix du filtre et comme il y a d'autres sujets sur le feu, j'imagine que cela sera tranché un peu plus tard. On peut essayer de faire un petit bouton discret (type libellé avec flèche qui part vers le bas) qui fait apparaitre le module de sélection directement dans le bloc du haut si l'on ne veut pas faire plusieurs endroits avec des filtres et si l'on souhaite garder un type d'affichage de filtre (j'essaie de faire une synthèse des remarques). |
|
Affichage mobile corrigé. Pour l'instant, je n'ai pas trouvé mieux en terme de libellé et de taille de texte. (cc @Floby qui l'avait signalé) J'ai également modifié pour mettre en dessous de la sélection du tri. Cela dit, n'oubliez pas l'importance de filtre pour les moins de 55 ans (et même si la France va sans doute suivre le chemin de l'Allemagne en autorisant ceux basés sur adénovirus pour tous). Ensuite concernant l'apparition du filtre, je propose 2 options (maquettes vite fait, bien loin en qualité de celles de @francoisBouchet :) ) :
|
|
merci @mccob , on a désormais les "Chronodoses" (doses du lendemain disponibles à tout adulte sans condition) qui viennent se rajouter à la problématique d'affichage dans le haut de page... donc je vais plancher sur tout ça et repasser par une phase créa pour prévisualiser comment tout ça peut fonctionner en bonne symbiose. |
|
Véran l'avait annoncé, c'est confirmé : AZ reste réservé aux plus de 55 ans : https://twitter.com/HAS_sante/status/1392529905087389701?s=19 (Pour le contexte du besoin de la fonctionnalité de filtrage par type de vaccin) |
|
on en avait aussi rediscuté lors du dernier "point prod" en webcam avec Florent & Frédéric semaine dernière. mais on a dû prioriser les chronodoses pour être prêts pour le 12 mai. alors je suis plutôt développeur du dimanche (1 fois par mois), et plutôt totalement ignare en TypeScript et lit-element et Bootstrap...
j'étais en train de regarder mon filter panel dans le template de la page résultat, histoire de vérifier les histoires de positionnement dans le template et comment il se comporte en responsive. Voilà où j'en suis. je n'irais guère plus loin. je pense que tu pourrais reprendre ces éléments "statiques" (soit maintenant, ou un peu plus tard après vérif responsive) et le greffer à ton code qui réalise la filtration dans la page résultat. Qu'en penses-tu @mccob ? |
|
Hello ! |
Hello, 👋 #233 est sur les rails et apporte un filtre 18-55. |
|
#233 a finalement été mergée / mise en prod en début de semaine .... mais le filtre par vaccin a finalement été supprimé de cette PR (une majorité du code a été conservé, mais le filtre n'apparaît désormais plus) Quelques insights derrière cette décision :
Il n'est pas impossible qu'avec l'arrivée de nouveaux vaccins (comme ceux de Sanofi en fin d'année), les stratégies changent. |
C'est une modification pouvant être vue comme impopulaire mais comme j'ai eu l'occasion de l'expliquer ici : #147
à l'heure actuelle les moins de 55 ans avec comorbidité doivent se faire vacciner en ARNm. Cette évolution permet de sélectionner la catégorie ARN messager afin de trouver plus rapidement un vaccin.
Il y a 3 points à vérifier :