NFT preview card with flexbox

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  hello @billbahr ,

  You are welcome the file path to the image is not right as the file path points to a file in the images folder located in the folder one level up from the current file style.css.

  content: url(../images/icon-view.svg); 
  

  Hopefully this helps

  Marked as helpful

  0
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted over 2 years ago

  Hello @billbahr,

  Congratulation on completing your first solution.

  I have some suggestions regarding your solution:

  • 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 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.

  • For any decorative images, each img tag should have empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images

  • Images must have alt attribute.

  • The avatar's alt , you can use Jules Wyvern.

  • 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.

  • Read more how to write an alt text

  • To use more semantic tags, You can use <ul> to wrap the class="market " , and in each <li> there would be <img > and <p>.

  • No need for all this <hr> , as you simply can use ``border-top: to the.class="artist"`

  • For the avatar's part , you may use <figure > and <figcaption>.

  • To center the card on the middle of the page , you can use flexbox properties and min-height: 100vh to the body :

  body{
  display:flex;
  align-items: center;
  justify-content: center;
  width: 100%; 
  min-height: 100vh
  
  

  • Add display: block to the image , that would remove the little gap under the image.

  .component .image>img {
          .............
          display: block;
  }
  

  • using min-height: 100vh instead of height: 100vh allows the body to to grow taller if the content outgrows the visible page.

  • 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

  ---

  

  P

  Bill Bahr•20

  @billbahrPosted over 2 years ago

  @PhoenixDev22 Thanks for your feedback! I actually did look at your solution when I was trying to figure out the hover effect, but had trouble getting this part to work:

  content: url(images/icon-view.svg);
  

  The teal background color would always fade in appropriately but the icon wouldn't appear.

  Anyway, your feedback is very helpful and I'll probably circle back to my solutions at some point and make sure I'm able to improve them, especially with making them more responsive and better for accessibility.

  0
• 

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  Good job with this challenge Bill. Keep up the good work

  In terms of your accessibility issue your image tag requires an alternative description hence simply insert the alt attribute into the image tag like this <img src="" alt=""> and then specify a description that matches your image.

  I hope this helps

  Cheers, Happy Coding 👍

  Marked as helpful

  0

  ---

  

  P

  Bill Bahr•20

  @billbahrPosted over 2 years ago

  @TheCoderGuru Thanks for your feedback! I really haven't been focused on accessibility issues at all with these challenges, and I need to be better about that.

  0