Html, Css , Javascript

Please log in to post a comment
Log in with GitHubCommunity feedback
- P@Rahexx
Good job with this challenge! Here is some feedback from me:
1. Bottom Corners
Bottom corners of your activities are a little smaller, because of that activity does not cover whole image and I can see a little of the picture in the background.
2. Responsive Design
Great job making this responsive website, looks good on different screen sizes.
3. Class Names
Great that you use BEM methodology, but I do not see for example
user
block, but you useuser__box
element. For me, it's better to give each element you are styling class names because you know exactly what you are styling, and when somebody checks your code, they know exactly what they're looking at. Check point 10 to see what I have in mind.4. CSS and ID
Your styles look clean and great, as well as your custom-properties, but there are a couple of things you should consider:
- Your
@media
query is usingrem
, so for starters, it is not readable for me. Rem is good for responsiveness, but using rem for breakpoints makes them not immediately readable. It makes me think about what pixels it is; I should know immediately what breakpoint this is. - Styling with
ID
may cause you a lot of problems with responsiveness, especially for more complicated designs.ID
is harder to override than a class, and because of that, it is suggested to style by class names, not by ID. With BEM, you should not need to use ID. Especially in the future, when you start using preprocessors, there will be rare occasions to use ID.
5. Destructuring
You could replace the code:
let filter = event.target.dataset.value; let label = event.target.dataset.label;
By using this:
const { target } = event; const { dataset: { value, label } } = target;
6. Const vs let
If you do not change the value of your variable, it should be declared using
const
, and if you change its value, it should belet
. It is recommended to use this way because if you do not have something to change, you will get an error in this case. Withlet
, you can accidentally change your value, which can cause some problems. This is the reason why, in point 5, I changed yourlet
toconst
.7. Console.logs
When you push something out, try to delete
console.logs
, because it is unnecessary to see them in production. They also make the code less readable.8. Inner HTML
Using
innerHTML
may be dangerous. It is not recommended to use this because it makes your code vulnerable to attacks like XSS, Data Injection, CSS Injection, Session Hijacking, Phishing, and DOM-based XSS. You can replace that by usingtextContent
orinnerText
. If you have to use this, consider using a library likeDOMPurify
to clean your HTML from dangerous elements and ensure proper validation of what you get.9. Fetch
Your fetch
fetch('./data.json') .then((response) => { if(!response.ok){ console.log('Something went wrong!') } return response.json(); }) .then((data) => { for (let elem of data){ let key = elem.title.toLowerCase().replace(' ', '__'); populate(key,label, elem.timeframes[filter].current, elem.timeframes[filter].previous) } })
Could be replaced like this:
fetch('./data.json') .then((response) => response.json()) .then((data) => { for (let elem of data){ let key = elem.title.toLowerCase().replace(' ', '__'); populate(key,label, elem.timeframes[filter].current, elem.timeframes[filter].previous) } }) .catch(err => console.error("Something Went Wrong", err));
With
then-catch
construction, let errors be handled by the catch statement if something goes wrong.10. For the Future
You have ten projects behind you, so think about learning SASS. It is a preprocessor for CSS, and it is more popular than other preprocessors like Less or Stylus. It has two conventions: I recommend you learn using both of them and after that choose the one you like more. Just keep in mind that SCSS (the second approach) is more popular than SASS. You will see the benefit of using
className
and BEM together.
Final Thoughts
Your website looks great and is close to the design. I know that I wrote a lot, but I hope that you will learn more thanks to my feedback and you will not be overwhelmed by it. Keep up the progress and happy coding!
Marked as helpful - Your
- P@arfath-ali
Good, but it still needs improvement.
Join our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord