Article Preview Component

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Desktop layout looks great, it is responsive and the mobile layout looks great as well.

  I think you already have the box-shadow applied on it, I tried looking at it and it seems it has it.

  Some suggestions would be:

  • First, it would be really great to have:

  html {
    box-sizing: border-box;
    font-size: 100%;
   }
  
  *,
  *::before,
  *::after {
    box-sizing: inherit;
  }
  

  As one of your default styling. By doing this, all element size will only be limited to its content or explicit sizes. Currently, without box-sizing: border-box an element's size gets changed with addition of padding and margin which should not be, using the styling I gave, it will be easier to handle element.

  • When using img element, never forget to add the alt attribute, either it is empty or has a value, include it. Not including this results to screen-reader reading the source path of the image which you don't want, so always include it.
  • I wouldn't really use h1 on the bold text since it doesn't look like heading title for me :>
  • Person's img should be using their name as the alt like alt="michelle appleton". When a person's name and image are both present in components like this, use their name as alt .
  • Social media toggle should be using button and not div. An interactive component needs to use interactive elements. Using div is not accessible.
  • The button that should be use on the toggle should have either aria-label attribute or screen-reader element inside it. The value for whatever method you will use should be the information on what the button does, for example on your button it should have an attribute aria-label="share on social media menu dropdown"
  • The img inside the button should be hidden so use alt="" and add aria-hidden="true" attribute on it. A decorative image needs to be hidden at all times.
  • On the social media, you could have use ul element to wrap all 3 since those are "list" of links and since they are links, they all should be using a tag and not only img.
  • Each a tag that wraps the icon again should either use aria-label or screen-reader method. The value will be the name of the link like aria-label="facebook" you set on the a tag. You typically do this if there are no text-content inside the element where screen-reader could have read.
  • Social media icon should be hidden since they are only decorative images.

  Aside from those, really great job on this one.

  Marked as helpful

  0