Pure HTML/CSS with Flexbox and Media Queries

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  Hello there, Skye! 👋

  Congratulations on finishing this challenge! 🎉

  I have some feedback on this solution:

  • Accessibility
    • The illustration is a decorative image, so could make the img has an empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images.
    • The alternative text for img element should not contain any words that related to image such as picture, icon, photo, illustration, logo, graphic, etc.
    • Some resources to learn about the alternative text.
      • Axess Lab | Alt-texts: The Ultimate Guide
      • Images Concepts • WAI Web Accessibility Tutorials
    • Chamu has said to you that the Register button should be an anchor tag. But, what do you think is going to happen if the user click that button? If you think it will navigate the user to a register webpage then it should be an anchor tag. However, if you think it will open a modal that contains form then it should be a button element.
    • In general, anchor tags are for navigation - moving to different pages or content on the same screen, while the button elements are for actions like opening a modal, submitting a form, toggling element, etc.
    • Always specify the type of the button. By doing that, you prevent the button from behaving unexpectedly (like submitting).
    • Every a element must have text content. In this case, since it only contains an icon, Chamu has recommended to you that you could have a visually hidden text inside it or you could just use aria-label attribute.

  * This is okay
  <a href="#">
    <span class="fab fa-facebook-f"></span>
    <span class="sr-only">Facebook</span>
  </a>
  
  * This is also okay
  <a href="#" aria-label="Facebook">
    <span class="fab fa-facebook-f"></span>
  </a>
  

  • Also, I would recommend using span instead of i element. Reference: https://github.com/hail2u/html-best-practices
  • Use rem or sometimes em unit instead of px. Using px will not allow the users to control the size of the page based on their needs.
  • Styling
    • Remove the width: 100vw from the body element. By default, the body element has already full width.
    • Don't limit the height of the body element, it will not allow the users to scroll the page if the page content needs more height. Use min-height instead.
    • I would recommend adjusting the breakpoint for the @media query. Also, I'm guessing that you are getting the 1440px from the style-guide and that's not what the value is for. It's just telling you that your solution should look something like this at 1440px (not making the site looks like this at 1440px). Your job as a developer is to make sure that your site looks good at all screen sizes.

  That's it! Hopefully, this is helpful! 😁

  Marked as helpful

  0
• 

  T

  Chamu•13,130

  @ChamuMutezvaPosted almost 3 years ago

  • the register button must be an anchor element. That should be a navigation to the registration site.
  • the anchor elements in the footer should have readable content, the images used are not providing information of the functions of the anchor elements to users. Use a sr-only class on a span which will be a child of the anchor element. Research on the sr-only class.

  1
• 

  Naveen Gumaste•10,480

  @NaveenGumastePosted almost 3 years ago

  Hay ! Good Job Skye

  These below mentioned tricks will help you remove any Accessibility Issues

  -> Add Main tag after body like it should be your container. For 1st heading or h1 tag, use header tag and then inside the header put your h1 or h2 etc . But use header tag only once in main heading element.

  Keep up the good work!

  0

  ---

  

  Vanza Setia•27,775

  @vanzasetiaPosted almost 3 years ago

  @Crazimonk It looks like you don't get what I've said to you earlier. Your trick might remove the "Accessibility Issues" that have been generated by Frontend Mentor, but that's not the only thing that matter.

  I would not recommend using header inside the main element because it doesn't have any meaning (role) once it lives inside the main element, so it behaves like a div.

  I would recommend changing your feedback to something like:

  All page content should live inside a landmark. Also, every page should contain at least one h1 as an identifier.

  Marked as helpful

  0

  

  Naveen Gumaste•10,480

  @NaveenGumastePosted almost 3 years ago

  @vanzasetia ok then let me try it with my own project first

  0