Responsive 3 Column Preview Component

Community feedback

• 

  Danilo Blas•6,300

  @Sdann26Posted over 2 years ago

  Soojeong you have done quite well however I don't recommend you only use div to enclose text, there are tags like <stromg>, <em>, <p>, <span>. I mention this because you have the name of the car brand in <div>.

  Regarding the <h1> you can solve it by placing the following tag with the following class: <h1 class="sr-only">Project title</h1>.

  Now the sr-only class will have the following attributes:

  .sr-only {
    position: absolute;
    width: 1px;
    height: 1px;
    padding: 0;
    margin: -1px;
    overflow: hidden;
    clip: rect(0,0,0,0,0);
    border: 0;
  }
  

  What this class does is to remove the title visually but keep it in the flow and detected by the screen reader :D!!!

  For last recommendation, when using media queries don't only look for the design to look good for 375px and 1440px, rather always keep in mind that in all screen sizes it looks good.

  If something is not understood you can consult me without any problem, I hope my comments help you :D!

  Marked as helpful

  1

  ---

  

  Soojeong•220

  @kongguksuPosted over 2 years ago

  @Sdann26 Thank you, Danilo, for your kind and thorough feedback. For the sr-only class, does this need a position: relative on its parent element? I'll definitely fix the tags for the car names and add more to media queries. Thank you!

  0

  

  Danilo Blas•6,300

  @Sdann26Posted over 2 years ago

  @kongguksu It doesn't need to be relative, don't worry :D!

  Good luck, you are on the right track!

  Marked as helpful

  1