Design comparison
Solution retrospective
help me to improve
Community feedback
- Account deleted
Hey there @Ali-Nash š
Good job š I have a few suggestions:
-
Always add descriptive alt text to images for accessibility.
-
Use rem instead of px for better scaling. Text scales better with rem than px because rem units are relative to the root element's font size, making them more adaptable to changes in font size across different devices and screen sizes.
-
Use the <main> tag instead of <div> for the main content to improve semantic structure. In your case, you can change it for the element with the class name of "content". This also helps the browser to better understand your code.
0 -
- @ZolfikaarPosted 9 months ago
- You can use padding for content div like so: padding: 50px 0 40px 0;
- You don't need to set the height for the content div, it will take the height by its content already unless you have limited space so you can use max-width to ensure that the content won't take more space than you want to.
- For colors, you can use CSS variables especially if you have a complex project that uses the variable value on different things (e.g primary color) like so: At the top of your CSS sheet define a root selector :root{ --primary-clr: hsl(75, 94%, 57%); --gray-clr: var(--Grey); }
.content h2 { color: var(--primary-clr); } by that way, if you have say 20 elements that use this (--primary-clr) value, and for any reason you want to change it you can do so from one place in the root selector.
- For the hover effect on links buttons you can add this: .content ul li:hover { background-color: var(--primary-clr); cursor: pointer; } .content ul li:hover a{ color: var(--gray-clr); }
0 - @faisalalwarePosted 9 months ago
Hey there @Ali-Nash.
Good job buddy! Your project looks nice!!
I have a suggestion about your code that might interest you.
You should work on box sizing, font size, mouse hover effect & responsive layout in your code. As compared to the design provided in the template your solution doesn't match completely, as it looks too big in size, no hover effect on links, not box sizing as per design and most importantly no responsive design in mobile view.
I hope this suggestion is useful for you.
Other than that, good job!
Keep Coding
0 - @danielmrz-devPosted 9 months ago
Hello @Ali-Nash!
Your solution looks great!
I have a suggestion for improvement:
- Use
<main>
to wrap the main content instead of<div>
.
š Tags like
<div>
and<span>
are typical examples of non-semantic HTML elements. They serve only as content holders but give no indication as to what type of content they contain or what role that content plays on the page.This tag change does not impact your project visually and makes your HTML code more semantic, improving SEO optimization as well as the accessibility of your project.
I hope it helps!
Other than that, great job!
0 - Use
Please log in to post a comment
Log in with GitHubJoin 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