AlvaroPrates
@AlvaroPratesAll comments
- @LokeshMunusamySubmitted 2 months ago@AlvaroPratesPosted 2 months ago
Hey, nice work! Pretty close to the design.
The only things that I noticed was:
- When opening the preview site in a wider screen, the component streches. I think this problem can be addressed by adding a
max-width
property to thecontant-hold
class. - Could decrease some
line-height
from the product title and increase someline-height
from the product description. But if you've done everything by eye, its pretty good.
0 - When opening the preview site in a wider screen, the component streches. I think this problem can be addressed by adding a
- @MisterWanerSubmitted 3 months ago@AlvaroPratesPosted 3 months ago
Hey, nice work!
Adjusts that make sense from viewing the screenshot:
- Add or increase the
max_width
property (withwidth: 100%
) - Change the bullet points color
- Adjust the font color to the lighter gray from the design system
Regarding the code, I saw that you nested css selectors. I didn't know that you could do that, thanks for showing me!
Marked as helpful0 - Add or increase the
- @Maxked-CodeSubmitted 3 months ago@AlvaroPratesPosted 3 months ago
Nice work!
Good use of css variables and code organization. Congrats!
0 - @Hart-wizSubmitted 3 months ago@AlvaroPratesPosted 3 months ago
Good work, keep on practicing!
Let's go over some point that I observed from the screenshot:
- I think the card component would look better if centralized in the page. You could achieve that by wrapping the card in a
<main>
or<section>
tags and setdisplay: flex;
and addalign-items: center;
andjustify-content: center;
. - Need to ajust the card
width
property. In the figma files the card's width is384px
. To make this adjustment easier, you can setbox-sizing: border-box
in a universal CSS selector. - In the design there is a little thin black border around the card. You can add all the properties in the
border
property. - Need to adjust the spacing between elements in the card information. I think the easiest way to achieve different spacing is with the
margin-bottom
property. - Probably should add a little more padding to the card, but I'm just being picky...
Marked as helpful0 - I think the card component would look better if centralized in the page. You could achieve that by wrapping the card in a
- @py-code314Submitted 3 months agoWhat are you most proud of, and what would you do differently next time?
I was able to make good use of Flexbox to center the card and make it responsive
What specific areas of your project would you like help with?- I wrapped card image, title and text in individual
div
s. Is it advisable to do it this way or is there a better way to do it? - Tried to use BEM nomenclature for class names. Please let me know if I made any mistakes!
- Little confused about the use cases of margins and padding. Is there any good resource out there which explains their proper use?
- Any alternatives to media queries so that I don't have to write them for every screen size?
@AlvaroPratesPosted 3 months agoI'm fairly new to web-development, but I think I can leave a little feedback.
When I've done this project, I forgot to add the
box-sizing: border-box
CSS property and had some trouble to adjust the width. So, nice catch implementing it!Regarding the topics that you need help:
- Instead of wrapping the image, title and text within normal
div
s, maybe you can use the<article>
and/or<figure>
HTML semantic tags. - I didn't use any media queries to this project. Do you really need any?
Marked as helpful0 - I wrapped card image, title and text in individual