Design comparison
Community feedback
- @correlucasPosted about 2 years ago
πΎHello @estebanp2022, Congratulations on completing this challenge!
Great code and great solution! Iβve few suggestions for you that you can consider adding to your code:
1.To make your CSS code easier to work you can create a
single classto manage the content that is mostly the same for the 3 cards (paddings, colors, margins and etc) and another class to manage the characteristics that are different (colors and icon), this way you'll have more control over then and if you need to change something you modify only one class.2.Add the website favicon inserting the svg image inside the
<head>.<link rel="icon" type="image/x-icon" href="./images/favicon-32x32.png">βοΈ I hope this helps you and happy coding!
0@estebanp2022Posted about 2 years ago@correlucas hi and thanks for the feedback!
I have looked at my code and I think thats what I've done so far - the article and .btn selectors have most combined properties. There appears to be a lot of repetitive code but its all specific since all sections have a specific color (and then again in their active states) so I can't group those.
Let me know if there was something specific you saw because I can't find where else to re-format haha! Also, what does the favicon do in the <head>?
Thank you!
0 - @VCaramesPosted about 2 years ago
Hey @estebanp2022, some suggestions to improve you code:
- Implement a Mobile First approach π±
With mobile devices being the predominant way that people view websites/content. It is more crucial than ever to ensure that your website/content looks presentable on all mobile devices. To achieve this, you start building your website/content for smaller screen first and then adjust your content for larger screens.
- To give you HTML code structure, you want to set up your code in the following manner (only did parent containers):
<body> <main> <article> <article class="sedan-card"></article> <article class="sedan-card"></article> <article class="luxury-card"></article> </article> </main> </body>The Main Element identifies the main content of the document.
While the Article Element will serve as the cardβs container, because the card represents a complete, or self-contained, section of content that is, in principle, independently reusable.
Lastly, each card will be wrapped in their own Article Element because they too, are complete, or self-contained, section of content that is, in principle, independently reusable.
More info:
https://web.dev/learn/html/headings-and-sections/
-
The headings are being use incorrectly. For this challenge, each heading is equally as important. So best option, is to use <h2> Heading, because it will give each card the same level of importance and it's reusable.
-
You want to use responsive properties to make you content responsive.
Happy Coding! π»π
0@estebanp2022Posted about 2 years ago@vcarames hi and thanks for the insight! I've re-worked the article tags as suggested.
I did not update the h1 headings to h2, however - when I've done that in the past, I get an HTML report issue that states that I cannot have an h2 tag without an h1 beforehand. Would you suggest otherwise?
Regarding the responsive properties - I've only coded for mobile (375px) for now.
Thank you!
0@VCaramesPosted about 2 years ago@estebanp2022
The report does not know that it is a component, which are meant to be reusable.
0
Please log in to post a comment
Log in with GitHub
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