p2p-Code-Review

[ITurres]

Hi @grauJavier

Your project looks very good with that personal design touch, great job!💪

⭐Highlights⭐

✅ Good commit history. ✅ ES6 syntax. ✅ Clear and readable variable/function naming convention. ✅ Well done using relative units. ✅ Formatted CSS and follows a convention (BEM).

Keep up the good work 🏆

---

Nevertheless, We can suggest the following;

Team Suggested Changes ♻️:

By @ITurres:

• 1) File: emptyMsg.js

  Given the following best practices, I quote:

  • "Don't use flags as parameters because they are telling you that the function is doing more than it should."

I would suggest the following: Rename this function to something like toggleMessageDisplay (or something else, also descriptive) and since the functionality within it makes the message classList be disabled or enable, therefore is toggling the class, So I will strongly suggest using the .toggle() method!😀 which toggles the presence of the CSS class d-none on the selected element. What do you think?

With the changes it would look something like this

const toggleMessageDisplay = () => {
  const msg = document.querySelector('#leaderboard__empty-msg');
  msg.classList.toggle('d-none');
};

• 2) File: getScores.js

[OPTIONAL]

When using the Fetch API, the GET method is the default method if you don't explicitly specify a method. So you could remove it from the fetch.

However, it's important to note that explicitly specifying the HTTP method (e.g., GET, POST, PUT, etc.) can still be beneficial for code readability and to ensure the intended operation is clear. It's good practice to explicitly specify the method you intend to use, even if it matches the default behavior.

So it is totally up to you 😀!

---

By @ITurres and @NitBravoA92:

As described here CSS Best Practices, it would be ideal to not overuse the !important rule in your CSS style definitions. However, We understand that you have done it to Override Bootstrap styles, you can modify your webpack.config.js file to ensure that your stylesheet is loaded after the Bootstrap CSS cdn stylesheet. This can be done by adding it as an entry point:

// webpack.config.js

module.exports = {
  // ...
  entry: {
    main: './src/index.js',
    custom: './src/custom.css' // Add your custom CSS file here
  }
  // ...
};

---

All in all, you have done a great job @grauJavier🎯

Thank you and happy coding 👨‍💻!

---

[grauJavier]

Beautiful CR @ITurres @NitBravoA92! Thanks a lot for your wise and concise suggestions. 🤗 Hugs!