Responsive landing page using Css Grid and Flexbox

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, awesome work on this one. Layout in desktop looks great, it is responsive and the mobile layout looks really great as well.

  Nam gave a great suggestion on this one, just going to add some suggestions as well:

  • I wouldn't add the website-logo inside the nav since it is not being treated as a link in here.
  • Website-logo img should be using the website's name as the alt like alt="huddle". Remember that a website's logo is meaningful so always make sure it uses the proper alt value.
  • Use button instead of input type="button" since there are more suitable elements to use.
  • If you remove the outline property of an element, make sure to add another custom visual indicator on the element's :focus-visible state. Try using tab on your keyboard to navigate the site, you will have a hard time since there is no indication on where you at.
  • Hero-section image should be hidden since it is just a vector image. Decorative image must be hidden at all times by using alt="" and extra aria-hidden="true" attribute on the img tag.
  • Also when using alt attribute, avoid using words that relates to "graphic" such as "illustration" and others. An img is already an image/graphic so no need to describe it as one.
  • When using heading tag, make sure you aren't skipping a level. If you use h3 make sure that h1, h2 are present "before" it.
  • Also, all images in the page are currently decorative images so better hide them by the method I mentioned.
  • Your cta section should have been inside the main element so that it is contained inside a landmark element.

  FOOTER

  • font-family is not being rendered make sure to check that one out.
  • Same for the website-logo it should use the website name as the alt value.
  • 3 icons on the 3 list of company information below the logo should be hidden.
  • Those 6 links should be contained inside a single ul element since those are related to one another and since they are links , make use of a tag. Interactive components needs to use interactive elements. You could wrap this ul inside a nav as well since it is still your website's navigational links.
  • Social media links could be inside ul since those are "list" of links and use a tag for those links.
  • Each a tag that wraps social media, it should have either aria-label attribute or screen-reader element inside it. The value for whatever method you will use should be the name of the social media like aria-label="facebook" on the facebook link a tag. This way, users will know where this link would take them.

  Aside from those, great work again on this one.

  Marked as helpful

  2

  ---

  

  Nirban Chakraborty•190

  @nirban256Posted about 3 years ago

  @pikamart first of all thank you for giving me feedback in such a detailed and helpful manner, and yeah I will make those changes from now on and thanks for telling me the aria-hidden option as I didn't knew about it before

  1
• 

  Nam Haϕ820

  @Nam-HaiPosted about 3 years ago

  Great work overall

  You might want to be a little more subtle on the shadows to match closer the orignial design Try to use a container and padding so that elements of you design don't "stick" to the side of the page. For exemple you could use your header section and add it padding: 2rem. It match closer to the original design too.

  The responsive design works great. Good job

  Marked as helpful

  1

  ---

  

  Nirban Chakraborty•190

  @nirban256Posted about 3 years ago

  @BlueTompon thanks for the advice i will do that

  0