Darko• 460
@Darko96
Submitted
Any feedback will be greatly appreciated 😊. Thanks
Carlos Martinez
@curlos
All comments
- Carlos Martinez• 310
@curlos
Posted
Hey Darko, here are my thoughts are on your project:
- Does the solution include semantic HTML?
- Yes, you included an h1 and appropriate p tags for the pitch and stats.
- Is it accessible, and what improvements could be made?
- It is very accessible with detailed CSS classes and you also had alternate text for the image describing it in great detail.
- Does the layout look good on a range of screen sizes?
- Everything looks good on desktop, mobile and tablet in regards to the actual card component. The only improvements that could be made here are reducing the space on the left and right sides surrounding the card. For mobile and tablet, it almost gives it a shrinking feeling with the large amount of space currently there. To be more accurate to the design, I would also add some more space at the top and bottom of the page.
- Is the code well-structured, readable, and reusable?
- The code is very well-structured. The CSS classes are very well named and reusable. You reused the same .box class for each of the three stats.
Good job on this project Darko! 🥳🎉
Marked as helpful
1 - Does the solution include semantic HTML?
- Max Kaiser• 300
@maxkaiser100
Submitted
Carlos Martinez• 310@curlos
Posted
Hi Max, nice project. Overall it looks pretty good and accurate to the design. Here are my thoughts on the project:
- Does the solution include semantic HTML?
- Your code doesn't really include any semantic HTML as it seems to only contain divs. For a simple project like this, the main one you could add are the heading tags like an h1 or h2 tag for the 'Victor Crest' name and stat numbers: "80K", "803K", and "1.4K".
- Is it accessible, and what improvements could be made?
- The project is accessible on mobile, tablet, and desktop screens. Some improvements that could be made (though very minor) are adding a box shadow to the card element since the original design had it and moving the circles slightly farther away from the middle of the card element (the left circle should be moved slightly to the left and the right circle moved slightly to the right).
- Does the layout look good on a range of screen sizes?
- It looks good on every screen as far as I can tell. A minor improvement that could be made is to make the max height the size of the screen so the scroll bar is removed. With this project, a scrollbar wouldn't be necessary unless the height of the screen is changed to something dramatically small. Also the footer at the bottom that says "Challenge by Frontend Mentor. Coded by Max Kaiser." should have either some margin-bottom or padding-bottom added to it as it sticks to the bottom of the screen currently and is practically unnoticeable on most certain screen sizes.
- Is the code well-structured, readable, and reusable?
- Your code is well structured and the CSS class names are very clear.
Anyways, there are just a couple of minor things to fix but you did a great job on this project Max! 👏👏👏
Marked as helpful
0 - Does the solution include semantic HTML?
- Kim Daniel Koch• 30
@Kimbo0o
Submitted
The map component did not automatically update the position by changing the values. I decided to remove and add the map when I update the position values. I would like to know if there is a better solution to achive the same behaviour.
Carlos Martinez• 310@curlos
Posted
Not sure what you mean by the map component not automatically updating the position? From what I can tell the map updates the position when a new value is entered. Also, I believe the IP Address that should show up on the page when the user enters should be their own IP address. Right now it is showing a different IP address and location to my own upon first entering.
You got everything else down for the most part. Maybe add a tooltip on top of the marker with some info like the location or IP address.
0
