NFT Preview Card Component Main

Community feedback

• 

  Naveen Gumaste•10,480

  @NaveenGumastePosted over 2 years ago

  Hello imen dhaoui ! Congo 👏 on completing this challenge

  Let's look at some of your issues, shall we:

  • To center the card vertically

  max-height: 100vh;
  display: flex;
  justify-content: center
  align-item: center;
  

  • Use the display-flex for the "Eth" and use the font-style and font-weight

  happy Coding😀

  Marked as helpful

  0
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @imendh02 ,

  Congratulation on completing another frontend mentor challenge.

  I have some suggestions regarding your solution:

  • Anything with a hover style in design means it's interactive. you need to add an interactive element<a> around the image , Equilibrium #3429 and Jules Wyvern.

  • For any decorative images, each img tag should have empty alt="" as you didand aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images in( icon-view, icon-ethereum, icon-clock ).

  • the icon-view doesn't really need to be in the html, you could do it with css. If you want it to stay in html it needs to be aria-hidden or role presentation with empty alt.

  • There are so many ways to add the hover effect on the image , The one I would use pseudo-elements to change the teal bg color to a hsla. Then opacity can be changed from 0 to 1 on the pseudo element on hover as there is no reason to have the extra clutter in the html.

  • The avatar's alt should not be "empty ", you can use Jules Wyvern. Read more how to write an alt text

  • the link should be wrapping the main image and either have Sr-only text, an aria-label or alt text that says where that link takes you.

  • You can use <ul> to wrap class="row" and in each <li> there would be <img> and <p> (not header).

  • To center the card on the middle of the page use flexbox properties and min-height: 100vh;` to the body . (no need for position absolute.)

  • width: 350px; an explicit width is not a good way . consider using max-width to card instead that will let it shrink a little when it needs to and a little margin to the card that will prevent it hitting the screen edges.

  • There's a little gap under the main image , give display: block that would fix this.

  • You should use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.

  General point : Really important to keep css specificity as low/flat as possible. The best way to do styling is single class selectors. Remember a css reset on every project. That will do things like set the images to display block and make all browsers display elements the same.

  You might have a look at my solution , to see how I did the hover effect on the image , it might my helps.

  Overall , your solution is good. Hopefully this feedback helps.

  Marked as helpful

  0
• 

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  Good job with this challenge Imen, keep up the good work.

  Your solution looks seemingly close to the design, in my opinion your cyan text which shows the price needs to be inline. You can fix this by adding a display: flex; to the respective css ruleset.

  In terms of your html validation error, all headings can only go up one level which means that your heading tags start from <h1> - <h6>. In my opinion I think you should either use a <p> or <small>

  I hope this helps

  Cheers Happy coding 👍

  Marked as helpful

  0
• 

  Travolgi 🍕•31,400

  @denieldenPosted over 2 years ago

  Hi Imen, I took some time to look at your solution and you did a great job!

  Also I have some tips for improving your code:

  • add descriptive text in the alt attribute of the image
  • Centering a div with absolute positioning is now deprecated, it uses modern css like flexbox or grid
  • use flexbox to the body for center the card. Read here -> best flex guide
  • after, add min-heigth: 100vh to body because Flexbox aligns child items to the size of the parent container
  • try to add a little transition on the element with hover effect

  Overall you did well :)

  Hope this help and happy coding!

  Marked as helpful

  0