Integrate multiple problems into existing flow

[alankbi]

List of Changes:

• Improve UI for switching between problems
• Display which problem the player is currently viewing
• Disable looping when reaching first or last problem in the game
• Update UI to better support Windows interfaces
  • Since I'm not on Windows, @jasonz6688 could you run this branch locally and confirm whether these changes helped or not? Namely I added a backup font for the editor and attempted to disable the scroll bar for the leaderboard when no scroll is needed
• Dynamically update numProblems based on the number of selected problems
• Remove mixing of selected problems and random problems functionality on the backend
• Disable problem selection/removal actions on the frontend when loading to prevent concurrency bugs
• Improve existing setup for copyable content
  • Required boilerplate is cut down on and the copied message correctly disappears 2 seconds after the latest click

Notes:

• The spectator page doesn't allow viewers to manually switch between problems, but I plan to implement this feature in the next PR (along with updating the leaderboard and results table setup)

Screencast and Images:

Game page with problem navigation:

[alankbi]

Integrate multiple problems into existing flow

[zpChris]

Really excited to review this, it all looks awesome! Going through #172 first, then I'll look at this 👍

[zpChris]

I have to head out a bit early today (I'm free all day tomorrow though), so wanted to provide some quicker feedback here:

1. When I ran the PR locally (using localhost:8080), I couldn't add more than one problem (it didn't automatically update the size for me?). Not sure why this might be, but that could be something to investigate.
2. I think the problem selection on the room page would be improved if it actually added them on the backend and loaded once you click "All good!", rather than on every new problem. Perhaps the problem selector doesn't even close when you click one problem, but that's not as important - I think having it add only on "All good!" would solve the delay problem and improve UX.

I'll have a fuller review tonight / early tomorrow morning, looks great!

[zpChris]

As always, this PR looks great! I left a few comments that mainly deal with potential improvements, but are not crucial to merging this PR - the only reason I didn't approve is because of my comments above on how the problem selection is not quite working correctly for me, for whatever reason.

I'll investigate a bit on my own setup to see what's happening locally, and comment here if I find anything. 👍 Excited to get this merged in!

[zpChris]

Upon more testing, it seems the error only happens with the admin account support@codejoust.co. I believe it may be fixed when just merging this PR with main to get the fixes introduced in #173 - if you end up doing that @alankbi, let me know if that works.

Edit: Upon further experimentation, I'm very confident this is the issue, it seems the frontend widget gets confused with the duplicate problems. 👍

[alankbi]

Upon further experimentation, I'm very confident this is the issue, it seems the frontend widget gets confused with the duplicate problems. 👍

Thanks for investigating this! Yeah, after merging it looks like the error doesn't exist anymore, so that should solve it

I think the problem selection on the room page would be improved if it actually added them on the backend and loaded once you click "All good!", rather than on every new problem. Perhaps the problem selector doesn't even close when you click one problem, but that's not as important - I think having it add only on "All good!" would solve the delay problem and improve UX.

That's a good idea - implemented! It did require a decent amount of extra logic and I deleted the remove button on the normal lobby page to make the modal the single source of truth for updating problems (and thus prevent any concurrency bugs hopefully). Feel free to test it out and lmk what you think (or if you notice any bugs lol)

[alankbi]

Just re-requested review, although there are still some merge conflicts which I might wait to resolve until #172 is finalized, and the commit history seems to contain extraneous commits for some reason (possibly due to differences in merging between this and stagingNumProblems). I'll make sure those are all resolved before merging.

[zpChris]

That's a good idea - implemented! It did require a decent amount of extra logic and I deleted the remove button on the normal lobby page to make the modal the single source of truth for updating problems (and thus prevent any concurrency bugs hopefully). Feel free to test it out and lmk what you think (or if you notice any bugs lol)

I love it! 💯 ❤️ I think this makes the process a lot easier, especially on localhost:3000 where the backend is slower to respond.

Slightly unrelated to this change but just general to the problem selector, I noticed that I was unable to close the problem selector list; I think this may have something to do with it being an element on the modal and not registering the location of other clicks, but I think it'd be helpful to have a "click outside" close that list.

There is also a slight note on whether selecting a problem should close that list or not - I'm split on this, so keep whatever you prefer.

[zpChris]

Given the issue with all the additional commits added, there are 56 files changed, so I'm not going to go through the list; I'm commenting without explicit approval with some other nits.

1. Compilation with warnings: there are frontend style warnings that you may want to fix (screenshot below of the run after npm start).

2. Socket Change Problem: This issue may be mute on your next PR, but I wanted to note that right now, on the spectator view, you can click the change problem buttons even though they don't do anything. (If this is irrelevant because of the next PR, don't worry about making a fix for this.)

3. Results Table Time: I think the Results table on the Spectator side shows "time" when it shouldn't - correct me if I'm wrong and it should be there.

4. Spectate Live Statistics: This may be another issue that you are fixing in the next PR, but I noticed that the spectator statistics in the live PR are not accurate; in the image below, I completely solved the first problem but not the second two, but the statistics still shows the score at 100% (even though I am on the second problem here). Whether it is scoring by problem or overall, this score is not accurate.

Outside of the above, this PR again looks great! Thanks for making the change on the problem title and back / forward buttons. 👍

[alankbi]

1. Compilation with warnings: there are frontend style warnings that you may want to fix (screenshot below of the run after npm start).

👍

2. Socket Change Problem: This issue may be mute on your next PR, but I wanted to note that right now, on the spectator view, you can click the change problem buttons even though they don't do anything. (If this is irrelevant because of the next PR, don't worry about making a fix for this.)

4. Spectate Live Statistics: This may be another issue that you are fixing in the next PR, but I noticed that the spectator statistics in the live PR are not accurate; in the image below, I completely solved the first problem but not the second two, but the statistics still shows the score at 100% (even though I am on the second problem here). Whether it is scoring by problem or overall, this score is not accurate.

Yup, both of these are features left for my upcoming PR on the NewLeaderboard branch!

3. Results Table Time: I think the Results table on the Spectator side shows "time" when it shouldn't - correct me if I'm wrong and it should be there.

Is this referring to the time column? If so, I wasn't aware we wanted to remove that (it should still be the same best submission time overall or per problem as of the NewLeaderboard branch). Or lmk as well if you were referring to something else

Slightly unrelated to this change but just general to the problem selector, I noticed that I was unable to close the problem selector list; I think this may have something to do with it being an element on the modal and not registering the location of other clicks, but I think it'd be helpful to have a "click outside" close that list.

Hm I'm also not able to reproduce this - do you have any steps for reproing? Currently running on localhost:8080 logged into the admin account

[alankbi]

Hm I'm also not able to reproduce this - do you have any steps for reproing? Currently running on localhost:8080 logged into the admin account

Although I will note when closing/reopening the modal and then clicking immediately to the left/right of the open problem selector, sometimes the first click doesn't register; however, this only happens occasionally and only for the first click for me

[zpChris]

Is this referring to the time column? If so, I wasn't aware we wanted to remove that (it should still be the same best submission time overall or per problem as of the NewLeaderboard branch). Or lmk as well if you were referring to something else

Yes, that's what I was referring to. I think it's inaccurate because if you are five minutes into the game, and click "Submit", it shows up as "5m ago" (when in reality, the submission occurred zero minutes ago but five minutes from the start of the game).

I made this screencast to explain what I see as the issues in the game, spectator view, and results table - sorry if it's confusing as I was gathering my thoughts during the screencast, but we can talk more about this tonight.

Hm I'm also not able to reproduce this - do you have any steps for reproing? Currently running on localhost:8080 logged into the admin account
Although I will note when closing/reopening the modal and then clicking immediately to the left/right of the open problem selector, sometimes the first click doesn't register; however, this only happens occasionally and only for the first click for me

I don't think the issue of closing / reopening the modal is a big deal; I just tested on localhost:8080 again right now and it worked great this time (who knows ¯_(ツ)_/¯), but I found a different issue: as shown by this screencast, when you close the modal it does not trigger the save problems, and nothing happens when refreshing the page either. I think ideally, closing the modal should discard the changes, and refreshing the page should also refresh the problems selected (however, if you fix the first problem the second isn't a big issue).

Let me know if this all makes sense @alankbi, sorry I keep finding little nits haha.

[alankbi]

I found a different issue: as shown by this screencast, when you close the modal it does not trigger the save problems, and nothing happens when refreshing the page either. I think ideally, closing the modal should discard the changes, and refreshing the page should also refresh the problems selected (however, if you fix the first problem the second isn't a big issue).

Good catch on this! I updated the close modal function to discard those changes if you exit without saving.

Yes, that's what I was referring to. I think it's inaccurate because if you are five minutes into the game, and click "Submit", it shows up as "5m ago" (when in reality, the submission occurred zero minutes ago but five minutes from the start of the game).

I made this screencast to explain what I see as the issues in the game, spectator view, and results table - sorry if it's confusing as I was gathering my thoughts during the screencast, but we can talk more about this tonight.

I see, thanks for clearing that up! Yeah that makes more sense (time showing wrong rather than the time column being removed) - I will note that in the PR description, I mentioned the following comment:

The spectator page doesn't allow viewers to manually switch between problems, but I plan to implement this feature in the next PR (along with updating the leaderboard and results table setup)

So I hadn't actually planned on making any changes related to the results/leaderboard in this PR but rather implement those in the next one (here's actually the commit that I made on that branch which fixes the mentioned time issue). If the concern is merging not-complete code into main, maybe one option is once both PRs are approved separately, we merge both in at once? Or if you prefer I can cherry-pick a few of the fixes into this PR before merging; lmk what you think

[zpChris]

Good catch on this! I updated the close modal function to discard those changes if you exit without saving.

Thanks! 💯

I see, thanks for clearing that up! Yeah that makes more sense (time showing wrong rather than the time column being removed) - I will note that in the PR description, I mentioned the following comment:

The spectator page doesn't allow viewers to manually switch between problems, but I plan to implement this feature in the next PR (along with updating the leaderboard and results table setup)

So I hadn't actually planned on making any changes related to the results/leaderboard in this PR but rather implement those in the next one (here's actually the commit that I made on that branch which fixes the mentioned time issue). If the concern is merging not-complete code into main, maybe one option is once both PRs are approved separately, we merge both in at once? Or if you prefer I can cherry-pick a few of the fixes into this PR before merging; lmk what you think

This all sounds good - I'm cool with you merging in this PR, and we can revisit these points on your next PR that you mentioned to double-check that everything is cleared up. Looks great!

[zpChris]

LGTM! ✅