Sunnyside - Mobile first with HTML & CSS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Layout in desktop seem fine, for the mobile layout, the sections that includes the fruit images is too large, occupying all the screen's height, toning it down would be really great.

  Some suggestions would be: DESKTOP

  • On the header lose the word "link" on the aria-label, since it is already a link, screen reader will announce it for you, aria-label="homepage" will work just fine.
  • The navbar could have used ul element, so that screen reader users will know how many links there are in the navbar, since screen readers announces how many items there are inside the list.
  • The alt text for the arrow-down could have been left empty alt="" since it is just a decorative image.
  • On the learn more links, you will notice that it takes the whole width of the its parent, try tabbing on it, you will see that the outline is at full width. Instead to prevent this, use width: max-content on the a tag. This will limit its width based on the content inside it, like it's supposed to do.
  • On the testimonial section, the name of the person could have been wrapped inside a heading tag. Why, because if I navigated through the testimonial section and I encounter the heading tag of the person, I would know that there a testimonial content is being binded to that heading, binding to that person.
  • footer sunnyside link as well could use aria-label="homepage".
  • The sunnyside logo in the footer should have sr-only text inside it so that it would have a text that will be read if screen reader navigate to it, if you want the svg to be hidden, use aria-hidden="true" attribute on the svg.
  • The 3 links in the footer could have been inside ul as well.
  • The social media links as well could have been inside ul element. The aria-label should only contain the name of the social media, like aria-label="facebook". aria-hidden on the social media links svg could be great as well.

  MOBILE

  • The hamburger menu should use a button element instead of a tag, because that hamburger menu is just a control element and not a link element right.
  • Hamburger menu as well should have been using aria-expanded attribute so that users will know if the dropdown has shown or not.
  • The img inside the hamburger menu should use alt="" because you don't want the user to navigate in that img. The button will create the sense.
  • Navbar on the dropdown as well should have been inside ul element.

  Aside from those, you did really great in here.

  Marked as helpful

  1

  ---

  

  Sinan•360

  @logistusPosted about 3 years ago

  @pikamart I did all the suggestions except fruit image sections.

  In order to show the text below the fruit images, I needed to add 150% padding-bottom. If there is another way to succeed, please tell me.

  Also, in the mobile design image, those sections have a 600px height. In my design at 375px width resolution, those sections have a 562.5px height.

  I'm not saying it looks good, it looks weird and way too long as you mentioned, but it is what I saw in the design image file.

  Thanks a lot for the suggestions.

  0

  

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  @logistus Hey, for that part, on the fruit-image-text , you are referring for mobile state right.

  Since you are using grid, you could have just set the height of those grid-items right. Making the custom size in the grid-template-rows property. I rarely use grid-template-areas too hard to handle I think.

  Using the grid-template-rows give the explicit height of the 2 fruit container at the bottom. That way, you won't need to add padding-bottom on it, because the grid parent will give it the proper height it need.

  0