CovidTrackerFr / vitemadose-front

Interface utilisateur de l'outil ViteMaDose
https://vitemadose.covidtracker.fr
Other
80 stars 51 forks source link

Ajout d'un onglet 'Carte' avec tous les RDV d'un département ou prés d'une ville #228

Closed Joxit closed 3 years ago

Joxit commented 3 years ago

Cette Pull Request est

Checklist

Description

Dans cette PR, j'ai mis en place un onglet pour changer entre une liste textuelle des RDV à proximité et une liste visuelle avec une carte des RDV à proximité.

La carte se place au bon zoom pour avoir une vue sur tous les RDV qui sont à moins de 50km ou de tous les points (dans le cas d'un département). Même si on zoom sur les RDV à 50km de l'origine, on peut toujours naviguer pour aller plus loin.

Screenshot

rozierguillaume commented 3 years ago

Merci beaucoup !!

rozierguillaume commented 3 years ago

@Joxit @fcamblor Visiblement il y a eu des problèmes lors du merge : https://github.com/CovidTrackerFr/vitemadose-front/commit/76224efe0851280a64a158ebb7d63a7602479273

bilelz commented 3 years ago

@Joxit @fcamblor Visiblement il y a eu des problèmes lors du merge : 76224ef

je check aussi, j'ai mergé une autre PR sans capter que la dev était déjà KO :-/

fcamblor commented 3 years ago

Euh ... est-ce qu'on pourrait éviter de merger des PR (conséquentes) qui n'ont été approuvées par personne de l'équipe dev ?

Joxit commented 3 years ago

Bonjour, voilà le correctif : https://github.com/CovidTrackerFr/vitemadose-front/pull/236

fcamblor commented 3 years ago

Pour info, on a une hausse de +30% du bundle de prod avec ce changement (lié au chargement du js+css de jawg dès la home), bundle js minifié multiplié par 2.4 (version gunzipée) qui passe de 59.8Kb à 144Kb

Vite_Ma_Dose___trouvez_un_créneau_de_vaccination_COVID-19 Vite_Ma_Dose___trouvez_un_créneau_de_vaccination_COVID-19
fcamblor commented 3 years ago

En toute honnêteté, j'aurais aimé discuter un peu plus de cette PR car elle va à l'encontre, en terme d'UX, avec ce sur quoi je travaille pour le 31 Mai (cf #233 ) car nous nous apprêtons à rajouter un bandeau permettant de sélectionner/filtrer le jour dont on souhaite afficher les créneaux.

Ce bandeau est assez incompatible (je trouve) avec l'utilisation d'onglets, ce qui me pose problème.

Je me demandais ... serait-il envisageable d'afficher la carte sous forme d'une popup suite à clic sur un bouton ? Cela interfèrerait moins avec l'UX qu'on souhaite mettre en place.

Joxit commented 3 years ago

Oui il est envisageable de faire une popup, mais cela ne changera pas le poids du bundle à la home. J'ai remarqué que la home chargeait tous les composants des pages des centres de vaccination même s'ils ne sont pas utilisés.... Vu que la popup restera un composant qui aura besoin de Leaflet, ça reviendra au même niveau poids :confused:.

Petite note, c'est bien le js et css de Leaflet qui sont chargés, pas de lien avec Jawg

fcamblor commented 3 years ago

ouip my bad par rapport à leaflet.

En fait, vitejs créée un bundle avec tous les imports situés dans le index.ts (ainsi que tous les imports transitifs) Et il est capable d'identifier des import dynamiques dans le code pour en faire des bundles séparés tout seul, sans qu'on n'ait rien à faire en dev (c'est magique :-) ) Pour info, chaque "vue" (=composant "racine") est chargée via un import dynamique.

Étant donné que dans le index.ts, on a tendance à référencer tous les composants sans trop réfléchir, si l'un d'entre eux (ex: vmd-appointment-map) fait un import de leaflet, ceci explique pourquoi leaflet est chargé dès le départ. Cela étant dit, je ne suis pas fortement en faveur de faire des modules séparés pour chaque vue, particulièrement les vues qui représentent notre traffic principal (en gros : home + resultats) : si on a besoin de leaflet sur la page de résultats, ça ne m'embête pas de payer son coût dès la home (car on arrivera dans 99% des cas de toutes façons sur la page de résultats). Ce qui m'embêterait, ce serait surtout de payer le coût de leaflet dès la home si on ne l'affiche que sur la page des centres (qui est très peu visitée par nos visiteurs).