-
Notifications
You must be signed in to change notification settings - Fork 9
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
refactor: Players #73
Conversation
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.
also pls format with prettier, some indents etc are off
I also alpha sorted the players available for teams addition, and make the Select Option available both when Adding & Editing Teams |
); | ||
setRefreshPercent(Math.min(100, Math.round((100 * offset) / players.length))); | ||
setPlayers(sortedPlayers(sort, players)); |
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.
doing these two setPlayers
calls back to back will run into the same async issue as before where it might not use the updated players
combine the two setPlayers
calls, for example:
setPlayers(
sortedPlayers(sort,
players.map((p) => {
const newPlayer = result.players.filter((r) => r._id === p._id)[0];
return newPlayer || p;
})
)
);
{mode === "teams" && isAdmin() && ( | ||
<Collapse> | ||
<Panel header={`Add new team`} key="1"> | ||
Type all player names separated by commas, with the captain's name first. |
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.
now that you don't need to type commas anymore, you could change the text to something like this
Add all player names, with the captain's name first
tourney: tourney, | ||
}); | ||
|
||
setTeams((t) => { |
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.
this isn't quite right -- i noticed that it fails to update the teams
it should be like this:
setTeams(teams.map((t) => {
Regarding #37 to refactor current state of the site, to migrate from Class Component based to use React Hooks
Known Issues currently:
message.error
like usual.Files Changed:
Additional notes: