@MattPahuta
Posted
Hi there. Nicely done completing this challenge. Your finished product is a close match to the design comp and you're using a lot of semantic element tags properly. Also good work with your use of custom properties and responsive units.
Regarding the naming of CSS classes, this is often a challenge for devs. For some good insight on the topic, check out this video from Kevin Powell.
Here are few other things I flagged:
- You should always have a
main
element on the page. Nest the div with the class name of 'main-container' within amain
tag. Then, for clarity, you might consider renaming the 'main-container' class to 'card'. - Since this is a card component that would sit along similar components on a page, it would likely never serve as the page title. Semantically, the heading should be an h2 instead of an h1 (even though the formatter will give you a warning about it).
- Your alt text for the image should just be 'Greg Hooper' or even something like 'Greg Hooper's avatar'. Image descriptions should not include words like 'image' or 'picture' because they are already an image role.
- You should have at least one fallback font where you have your font-family set.
- Remove that width from your card component ('main-container'). Give the body some padding instead and stick with max-width only on the card (but probably update that to use rem units instead of pixels).
- I think it's generally preferred to use unitless values for line-heights, but that might be debatable.
- You should avoid using percentages for margin and padding values. Stick with rems, ems, or even pixels.
Again, good work on this project. Good luck moving forward. Cheers!
Marked as helpful
@Divinitry
Posted
@MattPahuta
Hello Matt,
thank you a lot for the feed back I really appreciate it, will put these comments to good use. Will watch the video and put these tips to use, thank you!