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
Leaderboard: hit the API #2
Conversation
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 @bobb-Rob 👋,
Your project is complete! There is nothing else to say other than... it's time to merge it
Congratulations! 🎉
Highlights
- The Submit button hits the POST API point✔️
- The Refresh button hits the GET API point ✔️
- Use of arrow functions ✔️
- Use of
async
andawait
keywords along with thefetch
function ✔️
Optional Suggestions
- Kindly consider the optional suggestions. 👍
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.
As described in the Code reviews limits policy you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.
src/index.js
Outdated
const score = document.querySelector('#score-input'); | ||
|
||
const newUserScore = { | ||
user: name.value, | ||
score: score.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.
- [Optional] It would be nice to validate the name and score field so that a user won't be able to submit blank values. 👍
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.
Great Idea. I will do that right away. Thank you for this suggestion
src/index.js
Outdated
.then((data) => getId(data.result)); | ||
|
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.
- [Optional] Everything in the application works properly. However, it would be nice to use one
gameId
for your application. The reason I suggested this is that anytime I reload the page, all the scores are gone. This is because a newgameId
is fetched anytime I reload the page. For a good user experience, it would be great to have onegameId
. You can put instructions in theREADME.md
file to guide users on how to generate a newgameId
if a user wants to create a new game. 👍
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.
Fixed this issue
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.
@bobb-Rob, great work 👏
@Gambit142 Thank you so much for the help. Kindly check to confirm I implemented all suggestions. |
Implemented the following: