Second attempt.

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 3 years ago

  👋Hi Ted!

  I have some feedback on this solution:

  • For the class naming convention, I recommend to use BEM Naming Convention.
  • For the src value on every img tag. I would recommend you either add / (absolute file path) or ./ (relative file path). It will clarify where the images are. To learn more you can check my blog post about file path.
  • Only have one h1 on every page. Swap all h1 with h2.
  • Add width and height properties to every image for better CLS(Cumulative Shifting Layout). For more info, you can check web dev article about optimize CLS.
  • Avoid using !important flag. In this case you don't need it at all.

  a:hover{
    border: 2px solid var(--color-content);
    background-color: transparent;
    color: var(--color-content) !important;
  }
  

  • The body background color should be grayish color, not white.

  That's it! Hopefully this is helpful!

  Marked as helpful

  1

  ---

  

  Ted Dino•170

  @ted-dinoPosted about 3 years ago

  Hi @vanzasetia,

  I edit the HTML and CSS code. I would really appreciate it if you check it again and see if there is still I need to change/remove to make it more readable or easy to maintain. Thank you very much, your comment really helps me.

  0

  

  Ted Dino•170

  @ted-dinoPosted about 3 years ago

  and now it has an issue. lol

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted about 3 years ago

  @ted-dino Yeah, but it's okay in this project since it's not a full website. But, you can fix the issue by adding a hidden h1 and search "for the sr-only class styling" for the h1.

  <h1 class="sr-only">Preview Card Component</h1>
  

  It will visually hidden, but if you use your screen reader, it will still be able to read the text.

  1
• 

  Dharmik_487•1,740

  @Dharmik48Posted about 3 years ago

  Hello,

  Your solution is really good! Looks great! But just one thing you add is a transition on hover for the buttons, it will make the experience better.

  Keep up the good work!

  Marked as helpful

  1

  ---

  

  Ted Dino•170

  @ted-dinoPosted about 3 years ago

  Hi @Dharmik48,

  Yeah I think it would really look great if I add some functionality but my main goal is to make it look a little bit the same as the design provided inside the starter file and I'm still new to this Web Dev thing so I don't know how to do that. Maybe in the future I will add some animation/transition to make it more beautiful.

  Thank you for you feedback, really appreciate it.

  0

  

  Ted Dino•170

  @ted-dinoPosted about 3 years ago

  @Dharmik48 hey man, I updated the site. I add transitions in links. If you have time, please tell me what you think. Thanks!

  0

  

  Dharmik_487•1,740

  @Dharmik48Posted about 3 years ago

  @ted-dino Great!😄 It feels way better now!

  Marked as helpful

  1

  

  Ted Dino•170

  @ted-dinoPosted about 3 years ago

  thank you.

  0