HTML / CSS | Results-summary-component

Community feedback

• 

  Alexej Kunz•60

  @Orchi1904

  Posted over 2 years ago

  Hello ellenliao95,

  I am proud and happy to hear that you stuck with the challenge and did not give up. This is so important! Also I am happy to see your result. I am also only a beginner but I try to help you with some feedback.

  • The first thing is, as you can see in the accessibility report, <li> elements must be contained in a <ul> or <ol> like so:

  <ul>
     <li>
        //Your code          
     </li>
  </ul>
  

  In the case of this challenge, I would not even use <li> elements but rather use <div> elements. Here is an example of how I would structure the HTML without the <li> elements:

  <div class="container" id="bg-1">
    <div class="container-left">
      <svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" fill="none" 
               viewBox="0 0 20 20">
               <path stroke="#F55" stroke-linecap="round" stroke-linejoin="round" stroke- 
                width="1.25"
                d="M10.833 8.333V2.5l-6.666 9.167h5V17.5l6.666-9.167h-5Z" />
      </svg>
      <p id="li-1" class="li-space">Reaction</p>
    </div>
    <div class="li-num-2">80
      <span class="li-num-3"> / 100<span>
    </div>
  </div>
  

  This should be done for every container inside the "right-side" div. It will clean up your HTML a bit and fix the above <li> errors. You also have to change your CSS after a bit:

  .container{
      list-style: none;
      display: flex;
      justify-content: space-between;
      border: 1px solid none; //delete this, it does not change anything
      align-items: center; //add this, it will center the items inside the container vertically
      border-radius: 15px;
      padding: 12px;
      width: 200px;
      margin: 20px auto;
  }
  
  .container-left{
      display: flex;
      justify-content: space-between; //delete this, because container-class already has space-between
      align-items: center; //add this, it will center the svg and p vertically
  }
  
  

  • I also saw that you are not using the "Hanken Grotesk" font everywhere even though it is used everywhere in the solution. You could fix this easily by adding this to the body-selector in your CSS:

  font-family: "Hanken Grotesk", sans-serif;
  

  All the other font-family properties you used on other classes can than be deleted.

  • Your button uses the linear-gradiant background even though this should only be seen when someone hovers over that button. This can also be fixed very quick:

  button{
      width: 250px;
      height: 50px;
      border: none;
      border-radius: 25px;
      background: hsl(224, 30%, 27%); //use this color normally
      color: hsl(0, 0%, 100%);
      font-size: 18px;
      cursor: pointer;
  }
  
  //use linear-gradiant background, when somone hovers over the button
  button:hover{
      background:
      linear-gradient(
          hsl(252, 100%, 67%), 
          hsl(241, 81%,  54%) ) ;
  }
  
  

  • Also your design is not responsive, meaning it might look bad when somone visits your website with a smartphone. You can make your design responsive by using media queries. Normally you start designing your website for mobile first and than you use media queries to design it for bigger screens but you can also do it the opposite way. For example you could write something like:

  @media screen and (max-width: 720px){
   //Your code for small screens here
  }
  

  Inside this media query you can write CSS that should be displayed when somone visits your site with a screen-width smaller than 720px. Here is a explaination of media queries: Explaination on YouTube

  I hope I could help you with my feedback! Happy coding!

  04