Testing my knowledge

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Though I need to zoom out to see the desktop layout, are you using a wide screen? I am using a 1366x768. Though the desktop layout is great, mobile layout is great as well but your breakpoint is too much for mobile layout. You don't make all 0px-1440px a mobile layout, never, those are just the size where the layout is made and it is not related to breakpoint.

  Also there are lots of issues going on here.

  Some other suggestions would be:

  • Do not position: absolute a large container like the main. Inspect your layout, hover on the body you will notice that the body has no width because its direct child is out of the flow. Remove all :

  position
  left
  top
  margin
  

  On the main and in your body use :

  align-items: center;
  display: flex;
  justify-content: center;
  margin: 0;
  min-height: 100vh
  

  This will make the main always be centered and in the flow of the website.

  • Person's img should be using their name as the alt value like alt="Jeremy Robson" always use a person's name as the alt if both person's image and name are present.
  • I think you are using too much div to hold an element, there are some which you don't really need, maybe try to lessen some because you don't want a div nested elements. I am not saying its wrong but its hard to handle.
  • When using img always include the alt attribute otherwise screen-reader will read the source path of the image which you don't want. A decorative image needs to use alt="" and add aria-hidden="true" attribute on it.
  • When using heading tag, do not skip a level. If you use h5 make sure that h1, h2, h3, h4 are all present "before" it.
  • Those 3 dots should not be using button for now. The img inside it as well needs to be hidden using the method I mentioned above.
  • The date for each of those card should not a heading tag, use only p tag on it since it is not suited for heading tag.
  • Using button only on the selection is not accessible. On this one, you could use of input type="radio" for each of those,, since it is a selection, radio buttons are suited for those. You will be using a fieldset and legend so that the section will be a lot more accessible for users. I don't have a solution for this one so it's up to you to find about this.
  • Don't remove the outline if you remove this, you remove the default indicator of an element, if you do this make sure to add a styling on the :focus-visible selector of the element.
  • You don't need to create a separate whole html elements when a certain filter is selected. You could just use javascript to target each of the 6 items to change their value, this way you are not duplicating any element and being "dry".

  It would be great to refactor this. Aside from those ,great work still on this one.

  Marked as helpful

  0

  ---

  

  Jorge Merlin•75

  @JJM3RLINPosted about 3 years ago

  @pikamart Thanks, I did it

  0