Skip to content
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

Drag drop to create pools #177

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

SibiAkkash
Copy link
Contributor

@SibiAkkash SibiAkkash commented Nov 6, 2023

  • Create API endpoint to submit multiple pools
  • Drag/drop to create pools
  • Alert when pools are edited but not saved

If you update the seeding before submitting pools, the pools get reset to the state from the db.

drag-drop-pools-2023-11-12_14.50.18.mp4

Fixes #166

@Joe2k
Copy link
Contributor

Joe2k commented Nov 7, 2023

looks too good.. Will review in some time

})
);

setPools(allTeamsName, allTeams);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yo this will give wrong result when we have already created a pool, submitted and we refresh the page.

We will need to remove the teams which are already in the submitted pools.

Also well need to create the pools that were submitted before.. currently they will not show at all if we refresh after submitting

Copy link
Contributor Author

@SibiAkkash SibiAkkash Nov 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah i've fixed this now.
@Joe2k: if tournament is still in Draft, we should be able to edit the pools even after submitting right ?

If yes, maybe we can create the pool matches only when starting the tournament. Thoughts ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep so this is what we were talking in #180

So can you pick those changes also in this PR?


const [pools, setPools] = createStore({});

createEffect(() => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to change this to onMount as you want it run only on render

frontend/src/components/tournament/CreatePools.js Outdated Show resolved Hide resolved
@Joe2k
Copy link
Contributor

Joe2k commented Nov 8, 2023

Oh just read

This isn't complete yet, but just wanted to get feedback on this

Sorry for reviewing before xD
But yeah idea is super good and ui too... Go ahead and implement fully!

@Joe2k
Copy link
Contributor

Joe2k commented Nov 8, 2023

@SibiAkkash Lets try to complete this by Saturday if possible?... Comms will start creating tournament post sat afternoon. So be good to have this before that!

@SibiAkkash
Copy link
Contributor Author

Oh just read

This isn't complete yet, but just wanted to get feedback on this

Sorry for reviewing before xD But yeah idea is super good and ui too... Go ahead and implement fully!

Thanks @Joe2k yeah ;) this isnt' complete yet. I'll display the submitted pools above the drag drop, and remove them from allTeams list.

@punchagan
Copy link
Member

@SibiAkkash Lets try to complete this by Saturday if possible?... Comms will start creating tournament post sat afternoon. So be good to have this before that!

This is a nice to have, IMO. Please don't kill yourselves to finish things for Comms. Deadlines are good, but we don't want to be dead after meeting them!

@SibiAkkash
Copy link
Contributor Author

@SibiAkkash Lets try to complete this by Saturday if possible?... Comms will start creating tournament post sat afternoon. So be good to have this before that!

This is a nice to have, IMO. Please don't kill yourselves to finish things for Comms. Deadlines are good, but we don't want to be dead after meeting them!

no issues punch, shouldn't be a problem

@SibiAkkash
Copy link
Contributor Author

@SibiAkkash Lets try to complete this by Saturday if possible?... Comms will start creating tournament post sat afternoon. So be good to have this before that!

yup, working on it 👍

createdPools.push({
name: poolName,
seeding: poolSeeds,
sequence_number: index + 1
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here index will be with respect to just newly created pools right? But we need to make sure this index is unique with old pools too

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, while creating the pools store in TournamentManager, i'm adding the previously created pools also

// add previously created pools
    for (let { name, initial_seeding } of alreadyCreatedPools) {
        let poolTeams = [];
        Object.keys(initial_seeding).forEach(seed => {
          poolTeams.push({ seed, name: teamsMap()[tournamentSeeding()[seed]] });
        });
        setPools(name, poolTeams);
      }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is fine right ?

- The filter to check for invalid seeds was wrong, corrected this
- Lint files with new plugins
@SibiAkkash SibiAkkash marked this pull request as draft November 26, 2023 13:12
@SibiAkkash SibiAkkash self-assigned this Dec 2, 2023
@punchagan punchagan force-pushed the main branch 2 times, most recently from c60092f to f3e9311 Compare December 6, 2023 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drag drop teams to create new pool
3 participants