Second attempt.

Community feedback

• 

  Vanza Setia•27,715

  @vanzasetia

  Posted almost 4 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!

  13

  Marked as helpful
• 

  Dharmik_487•1,740

  @Dharmik48

  Posted almost 4 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!

  14

  Marked as helpful