@Mohamed-OdaySubmitted 4 months ago
Luam Belay
@LuamBAll comments
- @LuamBPosted 4 months ago
Hola 👋
congrats on your design, which is very close to the design!
Awesome aspects of your solution 🤩
- Use of CSS custom properties (variables)
- Use of transition
Stuff to improve 🤓
- You can define
font-family
once in the <body> element to avoid repetition and only definefont-weight
for each <h1> and <p> element:
body { font-family: "Figtree", sans-serif; ... } h1.title { font-weight: 800; ... } p.description { font-weight: 500; ... }
- In order to improve accessibility, you can use semantic HTML5 and you can use the BEM naming convention for your CSS classes to improve reusablility and code-sharing, e.g.:
<body> <main class="container"> <article class="blog-card"> <img class="blog-card__image" src="assets/images/illustration-article.svg" alt="#" > <ul class="blog-card__category"> <li>Learning</li> </ul> ... <figure class="author"> <img class="author__image" src="assets/images/image-avatar.webp" alt="#" > <figcaption class="author__name">Greg Hooper</p> </figure> </article> </main> </body>
- You can remove the
<div class="attribution">
element (included in the starter package of each challenge) from your final solution, because it's not included in the design.
I’m a beginner, but I hope my feedback is still valuable for you. I would appreciate if you could mark my comment as helpful if it was! 🙏
Let me know if you have feedback on my comment or any questions and I'll do my best to respond.
Happy coding! 💻
Marked as helpful0 - @Ankit04042001Submitted 4 months ago@LuamBPosted 4 months ago
- you might want to remove div.attribution
- you might want to improve spacing between img, h1 and p to match the design
- you might want to reduce border-radius of the img (border-radius of qr is ok, I think)
0