WIP- Little Review by Bocovo

[BOCOVO]

1. Missing Readme C'est important d'avoir un readme explicite dans son projet

2. Le dossier .vscode devrait être ignoré. Tu pourrais me dire ce qui t'a obligé à le conserver.

3. Irrégularité dans le message de commit Le type de commit est précisé des fois d'autre fois non. May be helpful

   

4. Des dépendances inutilisées Ces dépendances ne semblent pas être utilisées @radix-ui/react-dialog, @radix-ui/react-separator ...

5. Le href ne devrait pas être optionnel pour une bonne cohérence. https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/types/nav.ts#L3 Et ça t'éviterait d'ailleurs cette vérification https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/components/layout/site-header.tsx#L48

6. Mieux serait que tu crées le type et qu'ensuite tu l'utilises pour construire ton objet. Cela te permet de garantir que ton object suis un type bien défini et c'est plus sûr. https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/config/site.ts#L1

7. Tu utilises vraiment ceci ? https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/config/site.ts#L24-L28

8. Ce hook n'est pas utilisé et n'est pas explicite. https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/hooks/use-media-query.tsx#L3 Je pense que le nommé useMatchesMediaQuery est plus explicite, car il correspond au type de retours qui true ou false selon que la query est matches ou non. Et aussi la façon dont tu importes React n'est pas top. Import juste ce dont tu as besoin. Quelques ça optimise le sizing de ton build.

   import {useEffect} from "react"

9. No commented code and No unsed imports Il faut éviter les codes commenter, c'est vu comme sale. https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/lib/fonts.ts#L12-L15

10. Les modules lib/constants et lib/data ne sont pas des libs donc n'ont pas leur place dans le dossier lib. Tu n'as pas besoin du dossier lib dans ton projet, je trouve.

11. Tu peux créer un dossier utils et déplacer ce module lib/utils dedans, mais il faudra renommer le fichier ( maybe cn.ts ).

12. Pour une bonne organisation, je te propose de mettre tes images dans un dossier assets dans le dossier public.

13. Je propose de découper les icônes dans différents fichiers pour simplifier la maintenance. https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/components/icons.tsx#L1 L'autre risque est de finir avec des icônes non utiliser et augment la taille de ton build pour rien. Car tu importes l'object Icones qui inclura toutes les icônes dans ton build.

14. Mauvaise pratique. Cette directive use client que tu as ajouté vas rendre toute ta page client component. Alors qu'elle pourrait être constituée en bonne partie de RSC ( React Server Component ). Il faut le retirer et l'ajouter une uniquement ou c'est nécessaire. https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/app/page.tsx#L1 Tu peux extraire ce bout de code dans un autre composent et rendre ce dernier client component https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/app/page.tsx#L15-L22

15. Je ne sais pas pourquoi tu met toujours deux icones (. tu me diras ) https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/components/social-buttons.tsx#L26-L33

16. Je pense que l'utilisation du hook useTheme peut être éviter car tu as les class dark: de tailwinds pour appliquer des styles pour le dark mode.

...Pas encore terminé, je continuerai demain.

[vivienogoun]

1. Je suis conscient de l'importance du README. Je n'avais juste pas d'idée sur quoi mettre. J'ai ajouté quelques lignes maintenant (1767ec40c82677b4a16402738052c46930f2ba75)
2. Le dossier .vscode contient le fichier de configuration de l'éditeur pour le projet; plus précisement la configuration pour Typescript. Je n'ai pas de raison de l'ignorer puisqu'il n'est ni lourd ni ne contient des informations sensibles. Je l'ai conservé parce qu'il serait totalement utile à quelqu'un qui voudrait réutiliser le projet.
3. Remarque reçue.
4. Corrigé (145d3a84968b20299c7ab77f723859705c28077a)
5. Corrigé (aec709ac21c1aeb8e8373e33d0360275511cb683)
6. Corrigé (3361ff81b516a645eed9599683ea2ceb1ffc8d91)
7. Non. Corrigé (a36bfc67af49a3402c8f78a0a3b8b0962af93393)
8. Le hook n'est pas utilisé. Je l'ai supprimé (7bde998c4a3eeb7973d865f4814bd3c1ced33b31)
9. Corrigé (1e11c9a4809abf00c75f3147dee9641c0c81ea39)
10. Voir 11
11. Done (8c5fb7d80fe7502164c20ac221a1ff06cb98ae7e)
12. D'accord je prends ça en compte pour la suite. Je veux pas réécrire tous ces chemins maintenant ;)
13. Je prends ça en compte pour la suite
14. Corrigé (f9a4065941b18ae0dbff14aed760022cec535cbb)
15. La première icone est l'icone par défaut. La seconde icone est l'icone au hover (l'apparence de l'icone change)
16. Je ne l'utilise pas que dans les classnames tailwindcss. Je l'utilise à certains endroits pour faire quelque chose selon si on est en light mode ou en dark mode; comme par exemple pour les icones dans l'image du 15

[BOCOVO]

Suite des reviews @vivienogoun

17. Imbrication de condition ternaire https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/components/skill.tsx#L13 Cela rend ton code difficile à lire. Une alternative plus lisible serait l'instruction switch.

    const Skill = ({ name }: SkillProps) => {
    switch (name) {
    case SkillEnum.typescript:
      return (
        <TypesScriptIcon />
      )
    case SkillEnum.express:
      return (
        <ExpressIcon />
      )
    default:
      return null
    }
    }

    Tu vas remarquer TypesScriptIcon et ExpressIcon c'est que je te propose de créer des composants pour change icône de skill afin d'alléger ton composant Skill.

Une autre proposition est d'utiliser un objet key-value dont la key est de type SkillEnum et la value le composant de l'icone en question. Cette proposition va réduire davantage le poids ( en termes de nbre de lignes de code ) du composant Skill.

const SkillIconMap: Record<SkillEnum, React.Component> = {
  [SkillEnum.typescript]: TsIcon,
  // les autres composants ici
}

const Skill = ({ name }: SkillProps) => {
  return SkillIconMap[name]
}

[!WARNING] J'espere que je t'enbrouille pas 😆 . Tu peux adopter la premiere proposition pour faire simple. Tiens moi au courant si tu veux plus de précision.

• Au passage ton composant, il n'est pas bien nommé ( le composant Skill ). Il retourne l'icône convenable à un skill qu'on lui passe en props. Le nom Skill ne fait pas comprendre ça. Je te propose SkillIcon.
• Son props peut être mieux définie. Le name est de type SkillEnum plutôt.

  interface SkillIconProps {
  name: SkillEnum
  }

19. Same as above ( ou presque ) SocialButton

20. Pour assurer une bonne accessibilité, ajouter les types sur les boutons. Type submit pour celui de soumission et reset pour l'autre.

    • Tu as déjà un disabled={disabled} sur le button de submit, donc pas besoin de le retirer du DOM quand disabled est true. https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/components/layout/contact-me.tsx#L108-L114

21. Problème de bonne semantic. Tu n'utilises pas les bonnes balises. Pas de h ni de p. Tu fais une utilisation exagérer de balise div. Tout ce qui peut être considéré comme un titre doit être dans une balise h et tout ce qui peut être vu comme un corpus de texte dans un p.

[!> [!IMPORTANT] ] Attaches du prix au point 21.

22. Ton implémentation ici fonctionne, mais ... Il te faut nettoyer les timeout lors du démontage du composant. La actu ( en dev mode, car le strict mode est activé) tu dois avoir un timer de plus que voulu. Ton useEffect sera appele deux fois. https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/838cc60e17ed80bca1fbe3d1f3b221617968f690/components/layout/hero.tsx#L12

23. Les indexs en tant que key n'est pas recommandé. Tu en fais beaucoup usage. https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/838cc60e17ed80bca1fbe3d1f3b221617968f690/components/layout/testimonials.tsx#L16 Il faut que le key permette d'identifier une itération de façons unique. Tu peux voir ça comme un id. L'index n'en ai pas un. On peut en reparler si tu veux. Ce que fait d'hab faute de mieux est de composer de clé avec plusieurs autres valeurs. Par exemple, que peut faire key={`${index}-${testimonial.name}`} Ça permet d'éviter les incohérences de rendu possible, mais faire plus travailler React. ....

Conclusion

Il y a pas mal de corrections à faire.J'ai noté un gros défaut d'utilisation de bonne sémantique. Si tu devrais corriger une seule chose, que ce soit celui-là.

[BOCOVO]

@vivienogoun j'ai toujours pas compris ton explication sur le point 15

La première icone est l'icone par défaut. La seconde icone est l'icone au hover (l'apparence de l'icone change)

[vivienogoun]

Les icones sont par défaut en blanc sur noir et au hover, ça devient noir sur blanc. Je ne suis pas arrivé à gérer ça juste avec les classnames puisque les icones sont en svg et les couleurs sont directement dans le svg Donc j'utilise un prop hover pour changer les couleurs de l'icone selon l'état par défaut ou l'état hover Puisque je ne peut pas faire apparaitre le prop hover au hover (j'espère que c'est compréhensible) je mets deux icones; l'un étant dans l'état par défaut et l'autre étant dans l'état hover puis je fais apparaitre ou disparaitre l'un ou l'autre selon l'état que je veut afficher https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/components/skill.tsx#L20-L29 https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/lib/constants.ts#L1-L2

[vivienogoun]

@BOCOVO

17. J'avoue qu'une instruction switch case aurait été plus lisible. J'ai compris pour l'utilisation de l'object key-value; je prends ça en compte. Pour le composant Skill il ne retourne pas l'icone convenable à un skill; il retourne la card complète (icon + name + style) d'une skill qui lui est passée en props https://github.com/vivienogoun/FigmaToCode-Challenge-Week4-vivien/blob/06df5b3e901753214557697e04b01d1d5928e9ca/components/skill.tsx#L130 Peut-être tu n'as pas remarqué cette ligne C'est aussi pourquoi j'ai laissé le prop name en string. Mais je prend en compte la remarque
18. Corrigé. J'ai enlevé le disabled={disabled}. Je préfère faire disparaître le bouton pour ne pas agir négativement sur l'ui
19. Corrigé
20. Corrigé
21. Je comprends bien. C'est un peu de négligence de ma part. Je prends ça en compte Toutes les corrections ici 685d595494fa3747bf55209ed06e07e021ef4f72