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

Peer-to-peer code review with morning session team mates #2

Open
AbayomiOlaoye opened this issue Oct 20, 2022 · 0 comments
Open

Peer-to-peer code review with morning session team mates #2

AbayomiOlaoye opened this issue Oct 20, 2022 · 0 comments

Comments

@AbayomiOlaoye
Copy link

AbayomiOlaoye commented Oct 20, 2022

Hi @Ridwanullahi-code,

What a brilliant work you've done here, Champ! 👌 👌

To Summarize

  • You have an awesome UI implementation ✔️
  • Your code is clean and understandable ✔️
  • Your use of ES6 syntax is laudable ✔️
  • Great Git-flow ✔️
  • Your READMe.md documentation is very detailed with working live link included ✔️

Observations & Suggestions

  • Looking at your code, your attention to proper linting is undoubtedly commendable 👍
    However, the image provided below, as it is for every other tag in your index.html file indicates that you have 4-character spacing as your default code editor's tab size which is will save you a good amount of time and efforts fixing linter errors.
    <meta charset="UTF-8">

You can change this from keyboard settings by reducing 4 tab size to 2. It would be much conventional if you can effect this double-spacing principle. 🎯

  • Also, from the live demo version of your app, the input element for scores currently accepts data types that are not number (e.g. strings). This is a potential bug in your code especially if the input is needed for further operations 📝

To fix this, you could use client-side validation (since you haven't begun any back-end technologies yet). A good example would be to utilize JavaScript's access to Browser APIs, Regular Expressions, If...else conditionals or a much easier HTML 5 input semantic i.e. input type="number", to check authenticity before submission. 👍 🎯

  • I see that you are very much interested in Users' Experience as you take us through your code 😉 This is why I think if it would very much okay for user's (player's) interaction if he doesn't need to refresh or reload the game interface/page before his score appears. As we see, user's score only updates after a load event.

Potential fixes for this behavior would be to make a GET fetch request to update the UI elements immediately after the POST request initiated by form submission. You can equally use a callback function or the common setInterval(function, time in ms) 🎯

I will be happy if I can see the new version of your fully functional web app soon, and just in case you need me to clarify some stuffs, or need my thought as you work on these features, I'm just a click away on slack or you tag me here.

Cheers and Happy coding!👏👏👏

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

No branches or pull requests

1 participant