accordion-card

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Layout in desktop seems good, but the box is just a bit on the left side and the whole layout is not centered. On mobile state, the top part of the image gets hidden by the screen.

  Just some additions to what dewslyse already said:

  HTML structuring:

  • When you create a website, always have a main element that wraps the main content of that page like on this one. Having main element helps other users to navigate properly on your website.
  • The images could have used an empty alt text. When you think an image is just for decorative purposes only, you should make the alt to be alt="". This makes assistive tech to skip that image, because it doesn't really means anything.
  • On the FAQ section, using div alone as the toggler or container for those question is not really accessible or good at all. Since this is a dropdown, you can use the details element which is a native element that is really good for this kind of sections. It already have native accessibility that is why I recommend it. If you want another, you can make use button and have a aria-expanded attribute being set to it. You can search for the latter one. But details is enough.
  • The alt of the arrow icons on the dropdown should be left empty, alt="".

  For css

  • Don't apply any stylings in the html element. Don't. :>. Remove all of those stylings from the html aside from the font-size and box-sizing leave it there. The rest remove them.
  • I don't really recommend applying a fixed height and width on the body tag. On your case, better leave it at default, then just apply those fixed dimensions on the .project-container. To make it better, on your body tag, have these stylings:

  body {
    margin: 0; 
    min-height: 100vh; # do not use height: 100vw
    display: grid;
    place-content: center;  # centers the content;
    /*  add the linear-gradient background that you set in the html element */
    padding: # you can set how much padding to use, especially in top and bottom
   
   remove these stylings from the body:
      /* position: fixed; */
      /* top: 25%; */
      /* left: 25%; */
      /* width: 900px; */
      /* height: 500px; */
  }
  
   .project-container {  # this is where you should put the fixed dimensions. Also add a border-radius
     /* add the background image of the pattern in here */
       height: 500px;  
       width: 900px;
       position: relative;
       background-color: white;
       border-radius: 8px;
  }
  

  But remember, this is for the desktop layout. On mobile it will change, but at least you have a more clearer stylings. Sorry about that hehe:> , but hey, you can just tinker a bit on the mobile media query.

  Still, you did a good job in here. Also, you don't have to follow those css stylings that I mentioned, I just said it so that you will have an idea on where css should have. Like the html should not have any stylings, the body should now have those custom fixed dimensions. Refactoring css will be really good.

  Still, good work on this one.

  1
• 

  dewslyse•3,025

  @dewslysePosted about 3 years ago

  Hello @admase! Congrats on your submission. Looks good. The accordion items overflow the card when all items are opened. Aside that everything looks fine. You may also want to resolve the accessibility issues raised in the report. Happy coding

  0