-
Notifications
You must be signed in to change notification settings - Fork 4
Add color map to DirectoryItemInput so we can give different color by equipmentType and add Fields needed for contingency list by filter #839
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
Conversation
… equipment type and add new Fields for Aleas by filter Signed-off-by: basseche <[email protected]>
Signed-off-by: basseche <[email protected]>
Signed-off-by: basseche <[email protected]>
b0f47df
to
5b94394
Compare
Signed-off-by: basseche <[email protected]>
5b94394
to
bf01977
Compare
fields: elements, | ||
append, | ||
remove, | ||
} = useFieldArray({ |
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.
À mon avis il n'y a plus besoin de renommer fields ici vu qu'il est déjà renommé en bas, autant garder le nom d'origine ici plutôt que de le changer 2 fois
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.
ok renommage enlevé
item.specificMetadata?.equipmentType && | ||
equipmentColorsMap?.has(item.specificMetadata?.equipmentType) | ||
? equipmentColorsMap.get(item.specificMetadata.equipmentType) | ||
: undefined, |
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.
item.specificMetadata?.equipmentType && | |
equipmentColorsMap?.has(item.specificMetadata?.equipmentType) | |
? equipmentColorsMap.get(item.specificMetadata.equipmentType) | |
: undefined, | |
equipmentColorsMap?.get(item.specificMetadata?.equipmentType) | |
Corrigez moi si je me trompe mais je crois que du coup ça revient exactement a la même chose
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.
pas vraiment car si item.specificMetadata?.equipmentType est null ou undefined il y a possibilité de crash.
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.
Non ça renvoit juste undefined (j'ai testé avant d'écrire le commentaire :p )
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Map/get#return_value
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.
modifié
} | ||
/> | ||
{elementsWithMetaData.map((item, index) => ( | ||
<Box key={`Box${item.id}`} sx={{ display: 'flex', flexDirection: 'column', gap: 1 }}> |
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 pas quelque chose dont on risque d'avoir besoin ailleurs ? il faudrait peut-être sortir ce code et en faire un composant ?
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.
le but de cela est de mettre une couleur pour les chip et le type en HelperText en dessous. Pour le moment cette caractéristique n'est utilisée que par le formulaire des aléas. Je ne sais pas si on aurait besoin de ça ailleurs dans le futur. Le fait de faire une composant séparé j'y ai pensé, mais ça fait tout de même un certain pourcentage de duplication de code.
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.
Pourquoi duplication, quel code serait dupliqué ? Au contraire sortir le code en un composant permettra de limiter la duplication de code dans le futur, et d'alléger DirectoryItemsInput.
Mais bon pas grave au pire on pourra le faire dans une autre US quand on en aura besoin.
} = useFieldArray({ | ||
name, | ||
}); | ||
const elementsWithMetaData: TreeViewFinderNodeProps[] = elements as unknown as TreeViewFinderNodeProps[]; |
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.
On est sûr que les champs correspondent toujours à ce type TreeViewFinderNodeProps ?
J'ai essayé de remonter pour voir où se composant était utilisé et franchement j'ai toujours aucune idée d'à quoi ressemble ce formulaire.
En tout cas rien que le nom est bizarre, d'après le nom c'est censé être utilisé uniquement dans l'arbre mais je vois qu'il est un peu partout...
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.
normalement ça utilise DirectoryItemSelector, qui lui utilise TreeViewFinder les elements récupérer ont bien ce type.
src/utils/types/equipmentType.ts
Outdated
return ''; | ||
} | ||
return equipmentType !== EquipmentType.HVDC_LINE | ||
? (BASE_EQUIPMENTS[equipmentType as EquipmentType]?.label ?? '') |
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.
Pourquoi ne pas rajouter ce type d'équipement dans BASE_EQUIPMENTS ?
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.
parce que BASE_EQUIPMENTS est utilisé un peu partout je suppose et il ne faut pas modifier des listes uniquement pour notre besoin d'affichage de type.
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.
Pour moi il faut l'ajouter, sinon je vois pas l’intérêt de maintenir une liste d’équipement, si on commence à faire des if comme ça c'est pas maintenable, mais bon peut être que je me trompe...
@thangqp a créé cette liste, peut être peut il donner son avis ?
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.
autre enum utilisée finalement
Signed-off-by: Thang PHAM <[email protected]>
Co-authored-by: Thang PHAM <[email protected]>
Signed-off-by: Thang PHAM <[email protected]>
|
No description provided.