-
Notifications
You must be signed in to change notification settings - Fork 11
feat(rating): display the percent the 24h score and 1h score value changed compared to the previous one (+5%; +10%, -7% etc) #204
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
base: main
Are you sure you want to change the base?
Conversation
…anged compared to the previous one (+5%; +10%, -7% etc)
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.
Hi! Thank you very much for submitting this PR! 😊
I wonder why you had to update the dependencies. Can you please revert this change to keep the PR focused? You can submit it separately. It doesn't seem to be required to make the feature work.
|
|
||
| type ScoreWithChange = { | ||
| value: number; | ||
| change: number | null; // null represents an infinite change (from 0 to a positive value) |
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.
Let's not use null to denote special values. If you replace null with a string saying "infinity", the code becomes self-explanatory.
- let's also call it
percentageChangeto make it explicit that it's not an absolute value change.
| change: number | null; // null represents an infinite change (from 0 to a positive value) | |
| percentageChange: number | "infinity"; |
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.
One more thing. Have you considered returning value and previousValue and computing the change on the client?
This would simplify this type and the backend code and will make it a bit easier to follow.
| change: number | null; // null represents an infinite change (from 0 to a positive value) | |
| previousValue: number; |
| return "new"; // Display for changes from 0 | ||
| } | ||
| const sign = props.score.change > 0 ? "+" : ""; | ||
| // Use toFixed(0) for cleaner percentages unless you need decimals |
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.
| // Use toFixed(0) for cleaner percentages unless you need decimals |
| <td class="px-6 py-4 bg-gray-50 dark:bg-gray-800"> | ||
| {{ userRating.score24hours }} | ||
| <ScoreWithChange | ||
| v-if="userRating.score24hours" |
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.
Do we need v-if here? The score should always be defined, right?
| v-if="userRating.score24hours" |
| <td class="px-6 py-4"> | ||
| {{ userRating.score1hour }} | ||
| <ScoreWithChange | ||
| v-if="userRating.score1hour" |
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.
Ditto.
yurijmikhalevich
left a comment
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.
Thank you for the PR!
Can you please downgrade the dependencies? The update breaks our CI. The project is using Node v18.
If you want, you may submit a separate PR updating the project dependencies (including the Node dependency).
How does this PR impact the user?
Before :
After:
This feature provides players with immediate, at-a-glance feedback on their recent performance trends.
Performance Insight: Next to their 1-hour and 24-hour scores, players will now see a percentage indicating how that score has changed compared to the previous period (e.g., the last hour's score is compared to the score from the hour before that).
Intuitive Color-Coding: The percentage change is color-coded for quick interpretation:
Green for a positive change (improvement).
Red for a negative change (decline).
Gray for no change.
Context for New Activity: If a player has a score in the current period but had no score in the previous one, it is labeled as (new), clearly indicating recent activity. This adds a layer of motivation and helps players understand the impact of their recent games.
Description
This PR introduces a system to calculate and display the percentage change for player scores over recent time periods. The implementation is broken into two main parts:
The API endpoint /api/rating has been updated to calculate statistics for two new time windows: the period from 48-to-24 hours ago and the period from 2-to-1 hours ago.
A new helper function, calculatePercentageChange, was added to safely compute the difference.
The data structure for the UserRating type has been modified. The score24hours and score1hour fields are no longer simple numbers; they are now objects with the shape { value: number, change: number | null }.
A new reusable component, ScoreWithChange.vue, was created to handle the display logic. This component is responsible for showing the score value, the formatted percentage change, and applying the conditional (green/red/gray) text color.
The main rating table has been updated to use the ScoreWithChange component, passing the new score objects to it. This keeps the table template clean and declarative.
Limitations
No limitations - The PR is complete
Checklist