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

Fix for Rating range not set to max value when switching to a perf with provisional rating (Issue #12801) #12916

Merged
merged 27 commits into from Jun 5, 2023

Conversation

adityaj2003
Copy link
Contributor

This is a bug fix for issue #12801
I fixed it such that whenever a variant where rating is provisional is switched it sets slider to -500 and 500. Whenever a switch is made back to a variant where rating is not provisional, then it sets the slider to the previous stored value stored in JSON. This is my first time contributing so please help me become better by leaving detailed feedback. Thanks!

adityaj2003 and others added 18 commits May 26, 2023 00:49
…'t set to -500 and 500 when rating is provisional.
Changed ratingDifference Sliders to show -500 and 500 as default values when isProvisional
Changed such that when a user switches to a variant that doesn't have a provisional rating, then rating slider sets back to stored values.
Changed functioning such that it knows when the variant of the game is changed so that it resets to stored value.
Remove console logs
Lint issues with tabs and spaces.
Removed double negation.
Mixed tabs and spaces
Prettier formatting
Prettier Formatting
* master:
  New Crowdin updates (lichess-org#12914)
  upgrade play
  use play.mailer
  fix ApiMoveStream
  Revert "Metal BG for blog"
  scala code golf
  Update uap-scala to 0.15.0
  Update sbt to 1.8.3
  Update netty-transport-native-epoll to 4.1.93.Final
  Update google-auth-library-oauth2-http to 1.17.0
  Increase Firefox min version for coep:credentialless origin trial to 114
  Set first letter of every language to uppercase
Copy link
Collaborator

@ornicar ornicar left a comment

Choose a reason for hiding this comment

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

that's a lot of code additions to fix a minor UI bug, maybe it can be simplified?

@@ -149,9 +148,45 @@ export default class SetupController {
aiLevel: this.aiLevel(),
});

private savePropsToStoreExceptRating = () => {
if (this.gameType && this.store[this.gameType]) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

but then it won't save anything if this.store[this.gameType] doesn't yet exist? Which is not how savePropsToStore works. Is that intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to follow existing saveProps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to make a separate eventListener for variant as whenever we switch to a provisional rating variant, I had to change the ratingMin and ratingMax to -500 and 500 but not store the -500 and 500 (as it auto stores whenever ratingMin and ratingMax is changed) because whenever we switch to a normal variant I can set the slider to what the user had previously set as.

ui/lobby/src/setupCtrl.ts Show resolved Hide resolved
this.enforcePropRules();
this.savePropsToStore();
if (this.root.data.ratingMap && this.selectedPerf() && this.root.data.ratingMap[this.selectedPerf()].prov) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

duplicated code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The if statement checks are required because then it gives a null value error

// Check if the rating is provisional
if (ctrl.data.ratingMap[setupCtrl.selectedPerf()].prov) {
setupCtrl.ratingMin(-500); // Set ratingMin to -500 for provisional rating
setupCtrl.ratingMax(500);
Copy link
Collaborator

Choose a reason for hiding this comment

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

that code doesn't belong in the view layer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed code from view layer

@adityaj2003 adityaj2003 requested a review from ornicar June 2, 2023 19:28
* master: (59 commits)
  Revert "fewer dom text nodes and more css tricks"
  only feature swiss tournaments where players actually play
  scala tweaks
  scala tweaks
  New Crowdin updates (lichess-org#12949)
  tweak audio context handling
  voice mic selector
  prevent lichess.storage race condition on init
  moveCtrl is available
  remove ui/voice `let moveCtrl` global
  move voice toggle code out of the event listener
  mic.ts: if the audio context is suspended, wait for user interaction
  use OpaqueInt+Int=OpaqueInt
  Use fullMoveNumber when writing played moves
  Revert RendererActor visibility
  make RoundSocket round robin listen to 16 channels
  scala tweaks
  fewer dom text nodes and more css tricks
  only bind one click for all top players of a team battle
  remove superfluous team battle tag attr
  ...
@ornicar ornicar merged commit 0278cd5 into lichess-org:master Jun 5, 2023
2 checks passed
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.

None yet

2 participants