Responsive four feature cards

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, awesome work on this one. The desktop view looks fine I think, just needed those padding or gap between each card so that it will set like a boundary. Also, if you could adjust the box-shadow a little more to dark side, that would be really nice. The mobile state looks fine as well, just for the breakpoint, it is too early. Right now, you are using 0px - 1250px to show the mobile state which is really big for only the mobile state to occupy since the desktop view could use more of those screen time. Adjusting that would be nice.

  For some other suggestions, here are some:

  • For this, you should only have a main tag as the direct child of the body. Put the 2 heading tag inside the main since it is part of the main-content of the site.
  • For the h1, the br is not needed on that one. What you can do is that, add a max-width on the h1 so that it will limit the size and that will wrap the second text on another row. Just remember to add margin: 0 auto to center it.
  • The text after the h1 is not really a heading tag. Usually, a heading tag is not like a long sentence and it should describe or give information on what the section/part of the layout would contain. Use p tag on this one.
  • main tag doesn't need top use role="main" since it is already a main.
  • Also in general, adding a max-width on the body for example will make the site more consistent. If you zoom out on your screen, you will notice that the layout's content stretches along. max-width will prevent this one.
  • On each card, you could just replace the article by div tag since it just looks like a regular content of the page to which can't be reused on other pages where article content can be. A div would be fine and each of the heading tag will be enough to structure it.
  • Also, since you are removing the h2 after the h1, use h2 on the h3 on each card so that you won't skip a heading level.
  • For all the icons on the cards, hide them since they are only decorative images. Decorative images are just images that doesn't contribute to the overall content of the site. They should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if you are using svg instead of img tag.
  • Lastly, just keep in mind about the first suggestions about the padding and box-shadow ^^

  Aside from those, great job again on this one.

  Marked as helpful

  1

  ---

  

  Lorenzo•150

  @ingegnerlorenzoPosted almost 3 years ago

  @pikapikamart Hi, and thanks for your feedback! Seems I need to get back on work on this. Refining these details will be key to be a better front end developer! Thank you for pointing out these upgrades to be done!

  1