Cadoles/daddy
Cadoles
/
daddy
19
0
Fork 0
Code Issues 16 Pull Requests Releases Wiki Activity

Permettre de gérer les options proposées dans un DAD #19

Manually merged
tcornaut merged 6 commits from feature/options into develop 2020-08-31 15:32:40 +02:00
Conversation 0 Commits 6 Files Changed 6 +6 -6
1 changed files with 6 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 952b1b6a8d - Show all commits

12
client/src/components/DecisionSupportFilePage/OptionsSection.tsx
View File

@ -51,23 +51,23 @@ export const OptionsSection: FunctionComponent<OptionsSectionProps> = ({ dsf, up
} }
const onAddOptionClick = (evt: MouseEvent) => { const onAddOptionClick = (evt: MouseEvent) => {
var options = JSON.parse(JSON.stringify(state.section.options)) const options = JSON.parse(JSON.stringify(state.section.options))
wpetit commented 2020-08-31 14:45:36 +02:00
Review
Copy Link

JSON.parse(JSON.stringify(state.section.options)) est utilisé ici pour faire une copie ?

Pourquoi ne pas plutôt faire const options = [ ...state.section.options ]; ? Cette méthode est plus performante qu'une sérialisation/dé-sérialisation JSON du tableau.

`JSON.parse(JSON.stringify(state.section.options))` est utilisé ici pour faire une copie ? Pourquoi ne pas plutôt faire `const options = [ ...state.section.options ];` ? Cette méthode est plus performante qu'une sérialisation/dé-sérialisation JSON du tableau.
tcornaut commented 2020-08-31 15:14:19 +02:00
Review
Copy Link

Malheureusement la méthode de copie que tu cites ne lève pas l'interdiction d'écrire dans options, je n'ai pas trouvé d'autre méthode que JSON.parse(JSON.stringify(state.section.options)) pour en faire une copie modifiable.

Malheureusement la méthode de copie que tu cites ne lève pas l'interdiction d'écrire dans options, je n'ai pas trouvé d'autre méthode que JSON.parse(JSON.stringify(state.section.options)) pour en faire une copie modifiable.
var option = newOption("Décision", "", ""); const option = newOption("Décision", "", "");
wpetit commented 2020-08-31 14:46:15 +02:00
Review
Copy Link

Si il n'y aura pas d'assignation de nouvelle valeur à la variable, préférer const à var pour identifier assez vite les erreurs liées à l'écrasement de valeurs "constantes".

Si il n'y aura pas d'assignation de nouvelle valeur à la variable, préférer `const` à `var` pour identifier assez vite les erreurs liées à l'écrasement de valeurs "constantes".
options.push(option); options.push(option);
setState(state => ({ ...state, changed: true, section: { ...state.section, options }})); setState(state => ({ ...state, changed: true, section: { ...state.section, options }}));
}; };
const onOptionChange = (id: string, attrName: string, evt: ChangeEvent<HTMLInputElement>) => { const onOptionChange = (id: number, attrName: string, evt: ChangeEvent<HTMLInputElement>) => {
const target = evt.currentTarget; const target = evt.currentTarget;
const value = target.hasOwnProperty('checked') ? target.checked : target.value; const value = target.hasOwnProperty('checked') ? target.checked : target.value;
var options = JSON.parse(JSON.stringify(state.section.options)) const options = JSON.parse(JSON.stringify(state.section.options))
wpetit commented 2020-08-31 14:46:57 +02:00
Review
Copy Link

Idem qu'un peu plus haut.

Idem qu'un peu plus haut.
options[id][attrName] = value; options[id][attrName] = value;
setState(state => ({ ...state, changed: true, section: { ...state.section, options }})); setState(state => ({ ...state, changed: true, section: { ...state.section, options }}));
}; };
const onRemoveOptionClick = (id: string, evt: MouseEvent) => { const onRemoveOptionClick = (id: number, evt: MouseEvent) => {
if(confirm('Voulez-vous supprimer cette option ?')){ if(confirm('Voulez-vous supprimer cette option ?')){
var options = JSON.parse(JSON.stringify(state.section.options)) const options = JSON.parse(JSON.stringify(state.section.options))
wpetit commented 2020-08-31 14:47:30 +02:00
Review
Copy Link

Idem, const semble préférable à var ici.

Idem, `const` semble préférable à `var` ici.
options.splice(id, 1); options.splice(id, 1);
setState(state => ({ ...state, changed: true, section: { ...state.section, options }})); setState(state => ({ ...state, changed: true, section: { ...state.section, options }}));
} }