Flex Grid for NFT preview

Community feedback

• 

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  Hey Nazemrap, good job completing this challenge. Keep up the good work

  Your solution looks great however I think that you should consider the following

  • The font family of the card should be the one specified in the style-guide.md file

  • The user avatar needs to be a bit bigger

  I hope this helps

  Cheers

  Happy coding 👍

  Marked as helpful

  0

  ---

  

  Nazemrap•200

  @NazemrapPosted over 2 years ago

  @TheCoderGuru Yep, i saw that also when i uploaded, it was writen in the body{} but not on the different hx {} or p{}.

  all corrected thanks !

  0
• 

  SaakarX•320

  @saakarxPosted over 2 years ago

  Hello Nazemrap 👋

  Good effort on this challenge! and great feedback above, In addition to programming feedback I would suggest you to:

  • Read about semantic. Semantic elements lead to more consistent code, they are easier to read and improve accessibility.
  • Change the alt attributes for the #profilo, as they don't any extra context for screen reader users. Changing to alt="" will make screen readers skip those avatar images.

  Good luck with that, have fun coding! 💪

  Marked as helpful

  0

  ---

  

  Nazemrap•200

  @NazemrapPosted over 2 years ago

  @saakarx Thanks, true that i'm struggeling with all this semantic element, gonna try for the next challenge to keep it cleaner with the article you suggested.

  Thanks !

  0
• 

  Rémi Martineau•355

  @MartineauRemiPosted over 2 years ago

  Hey there ! Congrats on your solution, it's looking good :) If you want to align your icons with the text, do something like :

      #clocks{
          display: flex;
          align-items: center;
      }
      .clockpic{
          margin-right: 8px;
      }
  

  Just do the same for #ETH and you should be good to go. Here are a few leads for improvment:

  1. You should set width and height properties to your <img /> tags in your html file.
  2. maybe wrap your card with <main></main>
  3. Imagine you want to change the spacing between the border of the card and it's content someday. You would have to change margins in all your elements in your css. I suggest you put something like that:

      .card{
          padding: 24px;
          //the rest of your code
      }
  

  (and you get rid of the unnecessary margins in all your elements). That way, you would have to change only this particular padding, and save a lot of time.

  Hope this can help you. Keep up the good work ! Happy coding :)

  Marked as helpful

  0

  ---

  

  Nazemrap•200

  @NazemrapPosted over 2 years ago

  Just did the correction, worked perfectly ! Thanks you very much for your very detail comment, that was very helpfull !

  1