feat: memoriser certains champs dans le localstorage #43
Conversation
|
Indispensable ! |
src/js/form-util.js
Outdated
|
|
||
| const pdfBlob = await generatePdf(getProfile(formInputs), reasons, pdfBase) | ||
| console.log(profile, reasons) |
There was a problem hiding this comment.
Il faudrait peut être supprimer ça 😂
There was a problem hiding this comment.
C'était déjà là du coup dans le doute j'avais laissé. Mais j'ai retiré, on verra bien 🙂
There was a problem hiding this comment.
Je pense qu'il est risqué de stocker des données personnelles dans le localstorage.
Peut-être utiliser les mécanismes déjà intégrés dans HTML5 et votre navigateur ?
| const pdfBlob = await generatePdf(getProfile(formInputs), reasons, pdfBase) | ||
| console.log(profile, reasons) | ||
|
|
||
| ;[ |
There was a problem hiding this comment.
Je ne suis pas un expert de JavaScript donc je peux me tromper, mais est-ce que ce point-virgule n'est pas en trop ?
There was a problem hiding this comment.
Sans le point virgule, javascript croit que le [ est l'accès à l'élément d'un tableau qui serait retourné par la ligne au dessus
par exemple
console.log('test')
[1].forEach(e => console.log)
est interprété comme
console.log('test')[1].forEach(e => console.log)
et throw une erreur "cannot read property '1' of undefined"
There was a problem hiding this comment.
Ce ne serait pas plus logique de mettre les trailing semicolons... à la fin de la ligne ? Ou ESLint l'empêche ?
There was a problem hiding this comment.
Y a tout un débat à ce sujet de puis des années https://www.google.com/search?q=javascript+semicolon+or+not voir également https://prettier.io/docs/en/rationale.html#semicolons
There was a problem hiding this comment.
https://dev.to/adriennemiller/semicolons-in-javascript-to-use-or-not-to-use-2nli
TLDR: en gros, au moment de compiler le code, JavaScript détermine les endroits où il pense qu'il aurait dû y avoir des points-virgule. Sauf qu'il y a plein de cas où il se foire. Dans ces conditions, il me semble plus judicieux de mettre les points-virgule partout pour éviter les erreurs.
Bonus : ça rend le code plus accessible à ceux qui sont moins habitués aux subtilité du langage, et ça réduit un peu la complexité cognitive du code :)
Cela dit, il me semble plus sage de faire dans le cadre d'une autre issue pour éviter du bruit inutile sur cette PR (et ça sera plus propice aux discussions) :)
0xCAFEADD1C7
force-pushed
the
feat/memoriser-les-champs
branch
from
32dfa06 to
986db1a
Compare
|
Les changements me semblent bon, par contre il pourrait être envisagé d'ajouter un bouton permettant "l'oubli" et la sauvegarde manuelle des données. |
|
Je rajouterai un opt-in dans le formulaire pour demander le consentement de l'utilisateur à sauvegarder ses informations sur le navigateur, car certaines personnes peuvent être amenées à générer une attestation depuis un appareil utilisé par plusieurs autres. |
|
Dommage qu'un nouveau repo ait été créé au lieu d'avoir une nouvelle branche sur le formulaire d'attestation de couvre-feu existant, ça aurait évité du temps perdu de développement par @0xCAFEADD1C7 car c'est une feature que j'avais déjà développé (LAB-MI/attestation-couvre-feu-covid-19#4) avec un localStorage chiffré (via la lib secure-ls), une case à cocher pour choisir ou non d'enregistrer les données ainsi qu'un bouton pour effacer les données enregistrées. Il n'y avait donc plus qu'à l'adapter pour cette attestation, ce que @tar-gezed a d'ailleurs fait dans #58 |
|
Le fait de devoir re-taper le nom, prenom, etc. à chaque fois est peut être voulu comme une feature Nudge. Renforçant ainsi la perception du côté exceptionnel et solennelle de la sortie. Voir https://www.google.com/search?q=nudge+attestation+sortie Perso je vais me faire un petit truc custom |
|
Bon, le min int a préféré utiliser "sa propre" solution avec 1b8d795... |
|
rendue obsolète par 1b8d795, je close :) |
Cela évite d'avoir à re-taper certains champs à chaque fois :)