Skip to content

feat: memoriser certains champs dans le localstorage#43

Closed
0xCAFEADD1C7 wants to merge 1 commit intoLAB-MI:mainfrom
0xCAFEADD1C7:feat/memoriser-les-champs
Closed

feat: memoriser certains champs dans le localstorage#43
0xCAFEADD1C7 wants to merge 1 commit intoLAB-MI:mainfrom
0xCAFEADD1C7:feat/memoriser-les-champs

Conversation

@0xCAFEADD1C7
Copy link
Copy Markdown

Cela évite d'avoir à re-taper certains champs à chaque fois :)

@naholyr
Copy link
Copy Markdown

naholyr commented Oct 30, 2020

Indispensable !

const profile = getProfile(formInputs)

const pdfBlob = await generatePdf(getProfile(formInputs), reasons, pdfBase)
console.log(profile, reasons)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Il faudrait peut être supprimer ça 😂

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

C'était déjà là du coup dans le doute j'avais laissé. Mais j'ai retiré, on verra bien 🙂

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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)

;[
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 ?

Copy link
Copy Markdown
Author

@0xCAFEADD1C7 0xCAFEADD1C7 Oct 30, 2020

Choose a reason for hiding this comment

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

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"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ce ne serait pas plus logique de mettre les trailing semicolons... à la fin de la ligne ? Ou ESLint l'empêche ?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 0xCAFEADD1C7 force-pushed the feat/memoriser-les-champs branch from 32dfa06 to 986db1a Compare October 30, 2020 12:53
@paris-ci
Copy link
Copy Markdown

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.

@ker0x
Copy link
Copy Markdown

ker0x commented Oct 30, 2020

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.

@theblackhole
Copy link
Copy Markdown

theblackhole commented Oct 30, 2020

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

@yvele
Copy link
Copy Markdown

yvele commented Oct 31, 2020

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

@a2br
Copy link
Copy Markdown

a2br commented Nov 4, 2020

Bon, le min int a préféré utiliser "sa propre" solution avec 1b8d795...

@0xCAFEADD1C7
Copy link
Copy Markdown
Author

rendue obsolète par 1b8d795, je close :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.