Improve coding skills by creating an NFT preview card component

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Greetings, Terremoto! 👋

  Congratulations on completing your first challenge! 🎉 Your solution looks pretty good! 😀

  There are two things that can be improved.

  • Firstly, I would not recommend making the .attribution to be position: fixed because on mobile landscape view, to be specific 640px * 360px, the attribution is on the card element which may be disturbing for some users. Instead, I recommend making it part of the layout which means you can adjust the flex-direction of the parent element and add some margin-top to it to separate it from the card element.
  • Secondly, there should not be text in span and div alone; instead wrap the text with a meaningful element.

  That's it! Hope you find this useful! 😁

  Marked as helpful

  1
• 

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  Hey good job completing this challenge

  Keep up the good work

  Your solution looks great however I think that the image and card is not supposed to have a stroke .

  In terms of your accessibility issues simply wrap all your content between main tags

  I hope this helps

  Cheers Happy coding 👍

  Marked as helpful

  1

  ---

  

  Terremoto BCN•60

  @terremotoBCNPosted over 2 years ago

  @TheCoderGuru Indeed they had no borders, I just wanted to be a little creative and give it a little depth effect. XD

  In other way I am interested in the point of the <main> tag, I understand in a document there can only be one <main> tag and I had put one to wrap the card components in case there was more than one. The card component is an <article> tag.

  Do you mean that the same component card should be a <main> tag?

  Thank you very much for your help and support.

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @terremotoBCN The card element should only be wrapped with main element. However, I think using the article tag won't hurt the accessibility either. I believe that the article tag in this case behaves like a div.

  Usually, you want to only use article or section to give emphasize a section of the page and it needs to have a label (using aria-labelledby) which usually a heading tag.

  Marked as helpful

  1
• 

  Kamasah-Dickson•5,590

  @Kamasah-DicksonPosted over 2 years ago

  Your solution is great but remove every position property on the footer and add text align center to fix the footer from overflowing content on mobile. Good job keep coding👍

  Marked as helpful

  1