Responsive Landing Page Using Media Queries and Flex Box

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  Hey, awesome work on this one. The desktop layout looks great, though it would be really nice to have a bit more padding in general since the content right now are too spaced out compared to the design. When resizing the layout, there are issues since the layout gets too squashed. Try going into like 530px on your screen, you will see that the content are squished and the layout in the footer is destroyed. Though the mobile state looks fine.

  For some other suggestions, here are some:

  • Adding a max-width on the body along with margin: 0 auto. If you zoom out on your screen, you will see that the contents are going out of places and just occupy the full width. Adding the max-width will help the site's content layout to be placed properly and be controlled.
  • The header and footer should be living outside the main tag so that it will be treated as primary landmark element properly.

  <header />
  <main />
  <footer />
  

  • The primary header should only be containing the topmost part of the site where it includes the site-logo, site-links and other controls if there are. The hero-section should be left for the main tag. The reason is that, we want the header to be as reusable so that we could used it on other pages of our site. By including the hero-section in the header then we won't be able to reuse it now because we don't want the hero-section to appear on the pages again right.
  • When using img tag, you don't need to add words that relates to "graphic" such as "logo, illustration" and others, since img is already an image so no need to describe it as one.
  • If you somehow look again on the images that are used on this site, almost all images except for the site-logo doesn't really give any information at all, meaning these images only serves as decoration on the site. Decorative images 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.
  • Only use descriptive alt on images that are meaningful and adds content to the site otherwise hide the image for screen-reader users.

  FOOTER

  • Same with the company logo, use a more proper alt value.
  • Those 3 icons on the 3 information below the site-logo should be hidden as well using the method mentioned above.
  • Those 6 links on the right side, they could be placed inside the nav since those are still your website's navigational links right. Also, since they are all related links, using a single ul would be really nice:

  <nav>
    <ul>
      li*6
    </ul>
  </nav>
  

  • Each a tag that wraps the social-media icon should have either aria-label attribute or sr-only text inside it, defining where the link would take them. For example, you should use facebook as the value if the link would take the user to facebook.
  • Social-media image should be hidden since it is only a decorative svg so use aria-hidden="true" on it.
  • Lastly, just addressing the responsiveness issue on the site:>

  Aside from those, great job again on this one.

  Marked as helpful

  1

  ---

  

  Vaibhav Sachdeva•70

  @sachdevavaibhavPosted almost 3 years ago

  @pikapikamart Thanks a lot for taking out your valuable time and write such a descriptive and valuable feedback. I'll surely address the issues and update my code.😇🙏

  1

  

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  @sachdevavaibhav Glad that you find my review useful^^

  1

  

  Vaibhav Sachdeva•70

  @sachdevavaibhavPosted almost 3 years ago

  @martpika I have addressed almost all the issues you have mentioned. It would be great if you could have a look at it.

  1

  

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  @sachdevavaibhav Sure! Let me just take a quick review on it right now.

  • For the header part, instead of using the absolute on the div inside it, use the header itself to be the absolute so that it will have a height. Try inspecting the site, hover on the header and you will notice that it has no height because it's content is out from the flow.
  • The site-logo is still not using the site's name and is still using the logo as alt value which is not a good value since it is still related to "graphic".
  • Some id is still being used as selector on this, might want to change it to class instead.

  FOOTER

  • The site is not using a proper alt value, use the site-name as value.
  • For the 6 links, you can use the ul as suggested to nest the 6 links:

  <nav>
    <ul>
      li*6
    </ul>
  </nav>
  

  • Each of the a tag on the 6 links doesn't need the aria-label attribute. You use aria-label to element that has no text-inside it, this will make sure that screen-reader will instead read the aria-label. It is used as well to make some element be announced on what kind of content it has, it can be use on ul for example to signify that the list has links the site's links inside it.
  • Each a tag for the social-media needs to use the aria-label and not the svg, so grab that and place it on the a tag.

  Marked as helpful

  1

  

  Vaibhav Sachdeva•70

  @sachdevavaibhavPosted almost 3 years ago

  @pikapikamart Thanks a lot for the valuable feedback. I really learnt a lot from your feedback. I have fixed the issues (please review them) but I did not understand why you mentioned to use classes over id. Once again Thanks a lot!

  0

  

  Raymart Pamplona•16,100

  @pikapikamartPosted almost 3 years ago

  @sachdevavaibhav Hey, glad that you found my review usefull^^

  For the id, the reason is css specificity and reusability. id can only be used once on an element, this is why class are used so that styling could be reused by some elements. id as well gives high specificity to a selector, meaning a single .class could not override the stylings applied to an id selector.

  Marked as helpful

  1

  

  Vaibhav Sachdeva•70

  @sachdevavaibhavPosted almost 3 years ago

  @pikapikamart Thanks for the explanation. I am glad to learn from you!🤝

  1