Closed apibrac closed 5 years ago
apibrac
commented
5 years ago Tout le point des constantes que tu importe juste au dessus, c'est de les utiliser comme type (et pas une string justement)
L'idée c'est que si t'utilise des strings, tu peux avoir des erreurs quand tu les tapes, ou quand le projet devient gros tu peux avoir des répétitions alors qu'il faut que ça reste différent
Du coup en utilisant les variables tu corrige ou repère direct ces erreurs
apibrac
commented
5 years ago pas besoin du state si tu l'utilise pas et du coup pas besoin de constructor
apibrac
commented
5 years ago il faut que tu installe Prettier ou un truc comme ça pour que ça te split les lignes et tout c'est quoi ton éditeur de texte ?
apibrac
commented
5 years ago I'm not sure mais ça il faut le faire hors du component
direct dans les imports je pense il faut surtout pas de code qui puisse être chronophage ou computophage dans le render
apibrac
commented
5 years ago ça s'est pas bon il y a rien dans le dispatch un reste de quand ça marchait pas j'imagine ?
apibrac
commented
5 years ago Une fois que t'as réglé le commentaire précédent tu peux faire directement
const mapDispatchToProps = {changeAccountState: changeAccountState};"accountStateRedux" pas top comme nom pour une variable qui est justement de base dans redux si tu renomme en "accountState" ça te simplifiera pleins de trucs après
tu vois comment faire pour renommer dans tout ton repo d'un coup ?
apibrac
commented
5 years ago le this.props.theme partout c'est ni beau ni maintenable
pourquoi t'utilise pas la variable theme définie juste au dessus ?
sinon il faut mettre dans des variable que tu utilise dans ton display,
globalement il vaut mieux jamais avoir de this.props dans le return (ça s'autorise si vraiment il y en a que un ou deux genre pour un mini composant)
yolanlepibrac
commented
5 years ago mon éditeur c'est atom, mais du coup, j'aime bien que mes style soient en une ligne. Même si c'est des grandes lignes, une dois qu'elles sont tapées, je vais plus forcément les trifouiller. Du coup ca évite qu'a chaque
https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail Garanti sans virus. www.avast.com https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
Le sam. 10 août 2019 à 11:57, Pib notifications@github.com a écrit :
il faut que tu installe Prettier ou un truc comme ça pour que ça te split les lignes et tout c'est quoi ton éditeur de texte ?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/yolanlepibrac/movie-displayer/issues/1?email_source=notifications&email_token=ALAWDLZC3OAGRP7NMDFQO63QD2GJXA5CNFSM4IKZCH42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4AKKMI#issuecomment-520135985, or mute the thread https://github.com/notifications/unsubscribe-auth/ALAWDL7LYOGNTCZ437TAYHTQD2GJXANCNFSM4IKZCH4Q .
yolanlepibrac
commented
5 years ago Si j'ai : accountStateRedux:dispatch.accountStateRedux, J'en ai besoin pour récupérer mon thème qui est stocké dans mon state global
https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail Garanti sans virus. www.avast.com https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
Le sam. 10 août 2019 à 12:01, Pib notifications@github.com a écrit :
ça s'est pas bon il y a rien dans le dispatch un reste de quand ça marchait pas j'imagine ?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/yolanlepibrac/movie-displayer/issues/1?email_source=notifications&email_token=ALAWDL3JDCEQJMWAJ2D5B2LQD2GYBA5CNFSM4IKZCH42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4AKMGA#issuecomment-520136216, or mute the thread https://github.com/notifications/unsubscribe-auth/ALAWDLZOUOKWNV25VFKE3SDQD2GYBANCNFSM4IKZCH4Q .
yolanlepibrac
commented
5 years ago Justement je met Redux à la fin de mes variables globales, pour les différenciées de mes vairables dans mes composants. C'est pas bien du coup ? Non du coup, je sais pas comment tout renommer d'un coup mais je peux chercher
https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail Garanti sans virus. www.avast.com https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
Le sam. 10 août 2019 à 12:08, Pib notifications@github.com a écrit :
"accountStateRedux" pas top comme nom pour une variable qui est justement de base dans redux si tu renomme en "accountState" ça te simplifiera pleins de trucs après
tu vois comment faire pour renommer dans tout ton repo d'un coup ?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/yolanlepibrac/movie-displayer/issues/1?email_source=notifications&email_token=ALAWDL6H36GVHQYN7EZDYW3QD2HQVA5CNFSM4IKZCH42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4AKPQY#issuecomment-520136643, or mute the thread https://github.com/notifications/unsubscribe-auth/ALAWDLYDNKVNGANW5MKIZB3QD2HQVANCNFSM4IKZCH4Q .
yolanlepibrac
commented
5 years ago Là en fait, c'est parce que j'ai 2 choses différentes :
1/ ma variable theme qui est le thème courrant que j'ai stocké globalement dans redux (je l'utilise pour donner les couleurs de l'app) : var theme = this.props.accountState.theme ? ThemesItems[this.props.accountState.theme] : ThemesItems[0];
2/ Les boutons qui mes permettent de choisir quel theme j'utilise : mon bouton theme est créé par une boucle map() qui me map chaque theme, et pour chacun il place un bouton qui prend les couleurs suivant le thème : d'où le this.props.theme.name // this.props.theme.id (qui dépend vraiement du thème qui est mappé)
donc le theme. réfère au thème courrant de l'app, tandis que le this.props.theme réfère au thème du bouton qui est afficher et qui permettra de changer le thème courrant
https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail Garanti sans virus. www.avast.com https://www.avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2>
Le sam. 10 août 2019 à 12:10, Pib notifications@github.com a écrit :
le this.props.theme partout c'est ni beau ni maintenable pourquoi t'utilise pas la variable theme définie juste au dessus ? sinon il faut mettre dans des variable que tu utilise dans ton display, globalement il vaut mieux jamais avoir de this.props dans le return (ça s'autorise si vraiment il y en a que un ou deux genre pour un mini composant)
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/yolanlepibrac/movie-displayer/issues/1?email_source=notifications&email_token=ALAWDL2YEXSZYZY5XZCTMTDQD2HYLA5CNFSM4IKZCH42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4AKQZA#issuecomment-520136804, or mute the thread https://github.com/notifications/unsubscribe-auth/ALAWDL64X6ZISDGC3JVO3MTQD2HYLANCNFSM4IKZCH4Q .
apibrac
commented
5 years ago mon éditeur c'est atom, mais du coup, j'aime bien que mes style soient en une ligne. Même si c'est des grandes lignes, une dois qu'elles sont tapées, je vais plus forcément les trifouiller. Du coup ca évite qu'a chaque
j'ai 3 à 5 lignes de style, ce qui augmente vachement la taille des fichiers..Yes okay à toi de voir, la norme c'est de mettre une limite à je sais plus cb, 120 caractères je crois
Je trouve ça vraiment bien mais bon j'imagine ça dépend de ton goût là
ça évite que ça casse le style du fichier sur github et qu'on doive scroller horizontalement. Après si le fichier devient trop grand il faut le splitter en deux justement
AH : et les style il va falloir les mettre ailleurs, pas en dur comme ça, je re regarderai ça
apibrac
commented
5 years ago Si j'ai : accountStateRedux:dispatch.accountStateRedux, J'en ai besoin pour récupérer mon thème qui est stocké dans mon state global
Nope, il y a rien dans dispatch, ce qu'il y a c'est dans le state, mais tu le récupère en bas de ton ficher
connect(mapStateToProps, mapDispatchToProps) c'est dans le mapStateToProps que tu peux récupérer des trucs du state, le mapDispatchToProps juste tu inject les actions
apibrac
commented
5 years ago Justement je met Redux à la fin de mes variables globales, pour les différenciées de mes vairables dans mes composants. C'est pas bien du coup ? Non du coup, je sais pas comment tout renommer d'un coup mais je peux chercher <avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> Garanti sans virus. www.avast.com <avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2> Le sam. 10 août 2019 à 12:08, Pib notifications@github.com a écrit : … /src/Components/BoutonTheme.js@
f5d91a7#L45 "accountStateRedux" pas top comme nom pour une variable qui est justement de base dans redux si tu renomme en "accountState" ça te simplifiera pleins de trucs après tu vois comment faire pour renommer dans tout ton repo d'un coup ? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1?email_source=notifications&email_token=ALAWDL6H36GVHQYN7EZDYW3QD2HQVA5CNFSM4IKZCH42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4AKPQY#issuecomment-520136643>, or mute the thread <github.com/notifications/unsubscribe-auth/ALAWDLYDNKVNGANW5MKIZB3QD2HQVANCNFSM4IKZCH4Q> .
Non pas bien : parce que c'est censé être la même variable justement !
Juste elles ont des références, et des noms différents avec toi, parce qu'elle sont dans des environnements différents,
Mais en soit celle qui est dans redux, c'est celle qu'on veut récupérer dans les composant, donc elle peut avoir le même nom on veut justement que ça ait la même valeur
Mais comme on peut pas la récupérer directement on passe par le mapStateToProps, mapper en gardant le nom ça simplifie vachement la relecture
je sais pas en atom mais ctrl + shift + f je crois que c'est la norme, tu find et tu peux replace partout d'un coup
apibrac
commented
5 years ago Là en fait, c'est parce que j'ai 2 choses différentes : 1/ ma variable theme qui est le thème courrant que j'ai stocké globalement dans redux (je l'utilise pour donner les couleurs de l'app) : var theme = this.props.accountState.theme ? ThemesItems[this.props.accountState.theme] : ThemesItems[0]; 2/ Les boutons qui mes permettent de choisir quel theme j'utilise : mon bouton theme est créé par une boucle map() qui me map chaque theme, et pour chacun il place un bouton qui prend les couleurs suivant le thème : d'où le this.props.theme.name // this.props.theme.id (qui dépend vraiement du thème qui est mappé) donc le theme. réfère au thème courrant de l'app, tandis que le this.props.theme réfère au thème du bouton qui est afficher et qui permettra de changer le thème courrant <avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> Garanti sans virus. www.avast.com <avast.com/sig-email?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail> <#DAB4FAD8-2DD7-40BB-A1B8-4E2AA1F9FDF2> Le sam. 10 août 2019 à 12:10, Pib notifications@github.com a écrit : … /src/Components/BoutonTheme.js@
f5d91a7#L28 le this.props.theme partout c'est ni beau ni maintenable pourquoi t'utilise pas la variable theme définie juste au dessus ? sinon il faut mettre dans des variable que tu utilise dans ton display, globalement il vaut mieux jamais avoir de this.props dans le return (ça s'autorise si vraiment il y en a que un ou deux genre pour un mini composant) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#1?email_source=notifications&email_token=ALAWDL2YEXSZYZY5XZCTMTDQD2HYLA5CNFSM4IKZCH42YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4AKQZA#issuecomment-520136804>, or mute the thread <github.com/notifications/unsubscribe-auth/ALAWDL64X6ZISDGC3JVO3MTQD2HYLANCNFSM4IKZCH4Q> .
Yes okay bon de toute façon là c'est des goûts et des couleurs
Dans tous les cas pour que quelqu'un puisse comprendre c'est pas top d'avoir deux variables différentes qui ont le même nom au même endroit. Celle des props c'est pas logique de la renommer donc mettre un nom différent à l'autre c'est bien Quitte à ce qu'il soit un peu long, il faut surtout comprendre à quoi il sert en quoi il est différent
apibrac
commented
5 years ago @yolanlepibrac une fois que tu pense que t'as fini avec cette issue tu peux la fermer juste en dessous
Je te mets des premiers commentaires sur ton code Si ça te plait et que tu corrige tu me dis et j'en t'en referai d'autre différents