My second project on Frotend Mentor

Community feedback

• 

  Tesla_Ambassador•3,070

  @tesla-ambassador

  Posted about 3 years ago

  Hey Burrito Doggie! I am happy that you have finished your second project all on your own! 🥳 the accessiblity issues have already been addressed by @TheCoderGuru. Your CSS is much better seeing that you have obtained a clearer understanding of flex! I just have a few tips to point out.

  1. You could consider adding a padding around your card component instead of giving all those paragraphs margins therefore they won't cling onto the walls of their parent container (the card).
  2. Incase you apply align-self: center and it doesn't work, try setting a height attribute for the parent component of the items you would want to center. In your case, you can just wrap the card into a main tag like this:

  <main> <div class='card'>...</div> </main> Then you could set the height of main to "100vh". You will then be able to align your items to the center hence eliminating the need for you to use "margin-top". The reason why you need to set the hieight is because the default height property is auto and it sets the height to fit all the content within you div therefore, in some cases you may not be able to align your items on a vertical scale.

  31

  Marked as helpful
• 

  

  ApplePieGiraffe•30,525

  @ApplePieGiraffe

  Posted about 3 years ago

  🤩 LOVE IT 😍

  21

  Marked as helpful
• 

  Shashree Samuel•8,860

  @shashreesamuel

  Posted about 3 years ago

  Great job completing this challenge

  Keep up the good work

  Your solution looks great however I have some recommendations regarding the design aspect

  • The title and the description are supposed to be a heavier font weight, I recommend rechecking the style-guide.md file to be sure.

  In terms of your accessibility issues

  • Document should have one main landmark, this is caused by a semantic <maintag not being present in your markup, simply wrap all the content between the body element within main tags since it will identify that a main content exists thus resolving all the errors.

  In terms of your code, I think that the paragraph element for the title should be a h1 since it's like a heading in the card. This will resolve your second issue where your document needs a level one heading.

  Read more on html semantics.

  Hope this helps

  Cheers

  21

  Marked as helpful
• 

  Anna Leigh•5,135

  @brasspetals

  Posted about 3 years ago

  Hi Alyssa! 👋

  It’s so awesome to see you finally submit another solution! Sorry I’m a bit late to comment (been ill, but better now!). You’ve already gotten some great advice from Tesla Ambassador & Shashree. I agree that instead of the two margins on the image, you could simply add margin: 15px to the card, and then add any extra needed margin/padding for the text to the .paras div. Semantically the first paragraph should be some sort of heading (h1, h2, etc.) rather than a p tag, as Shashree already mentioned.

  The only "new" piece of advice I have is typically it's best not to have a set height for elements, letting the content determine height instead. This helps to avoid overflow issues. For example, try adding more text to the second paragraph, and you'll see that the text will soon overflow the card. However, if you remove the height from the card or switch it to min-height, the card will expand with the text.

  Again, great to see you submit another solution and looking forward to the next one! 😄

  11

  Marked as helpful