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

Reformatting automatch finder box #2749

Closed
wants to merge 5 commits into from

Conversation

LForchini
Copy link
Contributor

@LForchini LForchini commented Jul 9, 2024

Based on this forum mock-up
This PR depends on these changes in the goban repo

Proposed Changes

  • Expand front-end for a potential rapid automatch speed
  • Enable functionality for dynamic-ish speed displays (the text under 'Blitz', 'Rapid', 'Live'). This could change depending on byo-yomi/fischer options in the settings menu
  • Add a (current disabled) 'Play a Friend' button. I haven't gotten round to the functionality yet, but I'll work on that soon.

sc

Feedback is very much appreciated, even on small styling issues 😅
Also I've probably missed out something obvious in adding the rapid time setting, so I would appreciate a double-check!

Copy link

github-actions bot commented Jul 9, 2024

Uffizzi Ephemeral Environment deployment-53992

☁️ https://app.uffizzi.com/github.com/online-go/online-go+com/pull/2749

📄 View Application Logs etc.

What is Uffizzi? Learn more!

@LForchini LForchini changed the title Reformatting challenge box Reformatting automatch challenge box Jul 9, 2024
@LForchini LForchini changed the title Reformatting automatch challenge box Reformatting automatch finder box Jul 9, 2024
@anoek
Copy link
Member

anoek commented Jul 9, 2024

Aesthetically I like this. Organizationally, Correspondence is a speed setting and it would be weird to have to click that instead of the big Play button to play a correspondence game, so not sure I'm a fan of that.

I also don't want to merge a PR of a disabled button with the expectation that someone else will come along and implement a new interface for "Play a Friend" and enable it, but I do think that feature would be good so if it was implemented I'd be happy with that. Alternatively we can remove it for the time being until someone has the time or inclination to implement that feature end to end.


As for the backend, there are several things that need to happen to fully add a new rating band for Rapid, but I think we can dove-tail the long PITA stuff in with our upcoming ratings update that's tentatively slated for Q3/Q4. In the meantime I think we can get the automatch pool in relatively easily and just have those games rated as Blitz with an asterisk that we'll properly re-rate those in their own band during the next rating update.

@LForchini
Copy link
Contributor Author

Aesthetically I like this. Organizationally, Correspondence is a speed setting and it would be weird to have to click that instead of the big Play button to play a correspondence game, so not sure I'm a fan of that.

Thanks! Correspondence is already handled differently in that it you can have multiple requests going and it doesn't instantly throw you into a game. I can see the argument for putting with the rest, but the word 'Correspondence' might be a bit too long 😛

I also don't want to merge a PR of a disabled button with the expectation that someone else will come along and implement a new interface for "Play a Friend" and enable it, but I do think that feature would be good so if it was implemented I'd be happy with that. Alternatively we can remove it for the time being until someone has the time or inclination to implement that feature end to end.

I'll try and work on it soon, but I agree, it's probably best to split this out. I'll remove the button for now.

For the rapid introduction, I'd have to defer to you on it, but I'm happy to work any changes you might need. I can also split out the rapid stuff in this PR if you want to merge it separately from the automatch finder changes.

@pavelwatson
Copy link

pavelwatson commented Jul 9, 2024

Hey, that looks really nice! I'm a big fan of this design concept. I have a few thoughts and suggestions if you don't mind:

  1. Right now, the "Play" button and the speed/board size switches don’t stand out enough from each other. It feels like everything is a "Play" button, if that makes sense. Maybe consider making the switches a bit more transparent, or just outline them in blue instead of filling them? Not sure
  2. The layout looks great, but it feels a bit cramped to me. Maybe experiment with adding a bit more white space? It almost feels like I'm looking at a mobile version on desktop.
  3. One thing I'm curious about - how would this design work with the current feature of searching for multiple board sizes at once? What time setting would it show when multiple sizes are selected? I largely agree with spaceraven's point made on the forum that it would be much easier and less confusing to search just one board size at a time.

These are just some thoughts, of course. You've already done an amazing job with this redesign, and I really appreciate the work you're putting in!

While we are at it, maybe we should consider updating the Custom Request/Challenge menu too? It's not very user-friendly right now - too many settings thrown at you at once. Most options can be hidden behind an "Advanced settings" dropdown, similar to what Chess.com or Lichess do. I'd be happy to help with something too.

Also, @anoek, I don't want to be "pushy", but it would be great if you could share some thoughts on the ongoing forum discussion. The community is very engaged and excited about potential updates, any information would be much appreciated!

@anoek
Copy link
Member

anoek commented Jul 10, 2024

Aesthetically I like this. Organizationally, Correspondence is a speed setting and it would be weird to have to click that instead of the big Play button to play a correspondence game, so not sure I'm a fan of that.
Thanks! Correspondence is already handled differently in that it you can have multiple requests going and it doesn't instantly throw you into a game. I can see the argument for putting with the rest, but the word 'Correspondence' might be a bit too long 😛

I"m second guessing putting correspondence up there. It kind of feels like a second class citizen down below the giant Play button, but I'm not sure it belongs up there with the live game controls. I'm not sure what the right solution is.

@anoek
Copy link
Member

anoek commented Jul 10, 2024

While we are at it, maybe we should consider updating the Custom Request/Challenge menu too? It's not very user-friendly right now - too many settings thrown at you at once. Most options can be hidden behind an "Advanced settings" dropdown, similar to what Chess.com or Lichess do. I'd be happy to help with something too.

Revamping the custom challenge would be potentially welcome, but feels like probably that's a separate PR.

@LForchini
Copy link
Contributor Author

LForchini commented Jul 16, 2024

I've given the buttons some breathing room. I still have to give the buttons below the break some margin space as well, I'll be doing that soon.
Edit: it was bugging me so I quickly fixed it now

@pavelwatson To clarify point 3, it's admittedly some inconsistent behaviour, but you can search for multiple board sizes at once, but not multiple speeds. This mainly because there are speed specific options that I don't know how to deal with.

sc

Copy link

This pull request has been marked stale and will be closed soon without further activity. Please update the PR or respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the stale Issues or PRs that have been open for a long time with no activity label Aug 16, 2024
Copy link

This pull request has been automatically closed due to inactivity. Please feel free to re-open it if you still want to work on it.

@github-actions github-actions bot closed this Aug 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Issues or PRs that have been open for a long time with no activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants