-
Notifications
You must be signed in to change notification settings - Fork 1
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
Ajout d'un bouton dupliquer ligne sur saisie des espèces #98
The head ref may contain hidden characters: "dupliquer-ligne-saisie-esp\u00E8ce"
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dans les grandes lignes, la PR est bonne pour moi
Merci beaucoup !!
Et dans le détail, je pense que ça vaut le coup de regarder le DSFR
et voir pour cette histoire de signature de fonction onDupliquerLigne
scripts/front-end/components/SaisieEspèces/FauneNonOiseauRow.svelte
Outdated
Show resolved
Hide resolved
|
||
/** | ||
* @param {EspèceProtégée} espèce | ||
* @param {Partial<FauneNonOiseauAtteinte>} options |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
La fonction pourrait n'avoir qu'un seul argument qui serait de type FauneNonOiseauAtteinte
(vu que ce qu'il manque à options
, c'est l'espèce qui est séparé dans l'argument à côté)
Avec un peu de recul, j'ai l'impression que chaque ligne correspond aux types OiseauAtteint
, FauneNonOiseauAtteinte
et FloreAtteinte
Tellement que je me demande si le nom de fichier ne devrait pas le refléter plus explicitement (je n'ai pas de bonne idée pour le nom exact)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avec un peu de recul, j'ai l'impression que chaque ligne correspond aux types OiseauAtteint, FauneNonOiseauAtteinte et FloreAtteinte.
Oui ! Proposition de nom : OiseauAtteintEditRow
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ouais, allé !
Merci beaucoup !!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Je me permets de rouvrir cette discussion
Mon message initial avait 2 points :
- un sur la signature des fonctions
onDupliquerLigne
- un sur le nommage des fichiers (qui est maintenant corrigé)
Qu'est-ce que tu penses du premier point ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
C'est tout bon pour moi, sauf sur l'histoire de la signature
Je me demande si c'est que tu ne l'as pas vu ou si c'est que tu ne le considères pas important et je veux bien une clarification (ou une correction) avant de merger
...options, | ||
}) | ||
function onDupliquerLigne(oiseauAtteint) { | ||
oiseauxAtteints.push(oiseauAtteint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah ouais, voilà ! C'est ce que je me disais, ça simplifie plutôt bien le code (même si on ne parle que d'une ligne...)
Yes, parfait, c'est tout bon pour moi pour le code |
Yes say bon ! Je vais merger :D |
Cette PR ajoute un bouton sur chaque ligne des tableaux de la page de saisie des espèces.