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

feat: Limited the display of the overall leaderboard to the top 5 only #61

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

RusiruD
Copy link
Contributor

@RusiruD RusiruD commented Sep 30, 2024

Context

Closes #60

What Changed?

I limited the size of the leaderboard to only be 5 rows of data.

How To Review

Start the application including the backend and frontend and look at the homepage to see if the leaderboard displayed contains only 5 rows of user data.

Testing

I have created additional users to ensure none are added to the leaderboard.

Risks

NA

Notes

If an error occurs on start up saying " Module not found error can't resolve 'mdb-react-ui-kit' " ensure to do npm install in the frontend folder This is due to the footer commit.

Copy link
Contributor

@syoo881 syoo881 left a comment

Choose a reason for hiding this comment

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

A simple, yet crucial change!
Using a slice method to limiting the display of the overall leaderboard works effectively, and I see no big issues upon testing.

I will say though, there might be an alternative we can consider and discuss with our team:

Alternative

  • Possibly, instead of setting a hard limit to 5, we could have a box with all the top scores possibly up to 10~15, and have them just be scrollable.
    This pull request was made to address the issue of the leaderboard filling up the whole bottom portion of the page, but a scrollable box could be something to consider.

But otherwise, Good Job Rusiru! Looks great.

@syoo881 syoo881 merged commit d1fddb0 into SOFTENG310-G5:main Oct 3, 2024
7 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.

[FEAT] Limit the size of the overall leaderboard
2 participants