LokeshMunusamy• 20
@LokeshMunusamy
Submitted
P
AlvaroPrates
@AlvaroPrates
All comments
- PAlvaroPrates• 110
@AlvaroPrates
Posted
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-widthproperty to thecontant-holdclass. - Could decrease some
line-heightfrom the product title and increase someline-heightfrom 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
- MisterWaner• 80
@MisterWaner
Submitted
PAlvaroPrates• 110@AlvaroPrates
Posted
Hey, nice work!
Adjusts that make sense from viewing the screenshot:
- Add or increase the
max_widthproperty (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 helpful
0 - Add or increase the
- Maxked-Code• 120
@Maxked-Code
Submitted
PAlvaroPrates• 110@AlvaroPrates
Posted
Nice work!
Good use of css variables and code organization. Congrats!
0 - hart-wiz• 80
@Hart-wiz
Submitted
PAlvaroPrates• 110@AlvaroPrates
Posted
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
widthproperty. In the figma files the card's width is384px. To make this adjustment easier, you can setbox-sizing: border-boxin 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
borderproperty. - Need to adjust the spacing between elements in the card information. I think the easiest way to achieve different spacing is with the
margin-bottomproperty. - Probably should add a little more padding to the card, but I'm just being picky...
Marked as helpful
0 - I think the card component would look better if centralized in the page. You could achieve that by wrapping the card in a
- py-code314• 290
@py-code314
Submitted
What 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
divs. 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?
PAlvaroPrates• 110@AlvaroPrates
Posted
I'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-boxCSS 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
divs, 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 helpful
0 - I wrapped card image, title and text in individual
