Closed afranco07 closed 5 years ago
@afranco07 In createPlayer.js, Checking if a user is an admin should happen on the back end when executing an update to roster. It shouldn't be handled on the front end since if the user is not an admin that option should not show up.
In help.js, we wrap all our JSX attributes in double quotes.
@afranco07 Sorry for the massive delay in housekeeping with PRs! I still have to test this locally but It looks good at first glance!
Saying that I noticed you changed .jsx
to .js
for that one component.
What was your reasoning?
Should we make another PR to change the rest of the components?
@afranco07 Thank you for this!
@paulywill You're welcome and thank you for reviewing this! Just wanted to point this out from above
Checking if a user is an admin should happen on the back end when executing an update to roster
This was the reason why this wasn't merged earlier and something I haven't done yet, just giving a heads up.
I noticed you changed .jsx to .js for that one component. What was your reasoning?
I don't quite remember since it was such a long time ago, but I think it had to do with there's no specific reason to use .jsx
over .js
now since the tools we use can handle jsx
like syntax in regular .js
files. I don't think we would need to change all the existing files since I don't think there is any benefit to changing them (and it would be more work), and in hindsight I should've left it as .jsx
so it would be consistent with the rest of the codebase.
I could be completely wrong about my reasoning however, so do double check out what I said about jsx
vs js
.
Thanks again!
For #80 , when admin adds player it is immediately added to the list of all players.