Responsive landing page with curved sections

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hi! 👋

  I notice that the font family is not the same as the design. So, my first recommendation is to use the correct font family.

  Here are several suggestions from me.

  • I suggest wrapping the logo img element with an anchor tag. It's a common thing that the logo will navigate the user to the home page. For example, clicking the Frontend Mentor logo will navigate you to the /home page. But, if you are not logged in, it will navigate you to the root page (https://www.frontendmentor.io/).
  • I would expect the "Try It Free" button as a link element. As a user, I think the button is going to navigate me to another webpage. The same goes for the other buttons.
  • header should not contain the page content. So, I suggest moving the page content inside the main element. As a result, the only things that live inside the header are the logo and the "Try It Free" button.
  • I suggest leaving the alt="" empty for all decorative images, like the illustrations, background images, and icons. A page full of decorative images with excessive alt-text adds noise to the page.
  • Always specify the type of the button if you ever need to use it.
  • Add aria-label to the social media links. Screen reader users won't be able to know those social media links if there's no text content or label.
  • Remove all the styling from the html element. Setting max-width: 100vw and font-size: 16px is not necessary because by default the page is 100% width and has 16px. Also, it's not recommended to change the html or :root font size. By doing that, you won't allow the user to change the font-size based on their needs.

  For your information, the sizes on the style-guide.md have nothing to do with the media queries. They are telling you that "this is how your website should look like at these screen sizes". As frontend developers, we should keep making your website looks good in between those screen sizes.

  Currently, on mobile landscape view, the site is not looking well.

  That's it! Hope this helps.

  Marked as helpful

  1

  ---

  

  Robin Dela Cruz•80

  @robin-dcPosted over 2 years ago

  Hi @vanzasetia 👋, I would love if you could review my coding techiques so I can improve more. Thank you!

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @robin-dc Sorry, what do you mean by "coding techniques"?

  0

  

  Robin Dela Cruz•80

  @robin-dcPosted over 2 years ago

  @vanzasetia, suggestions or recommendation for my html and css codes😊

  0

  

  Andrew Ezeani•370

  @ezeaniiandrewPosted over 2 years ago

  @vanzasetia Thanks for the feedback, it was really helpful and I have implemented most of it. Please, I'm having problems making the desktop view on mobile responsive any tips on how I can go about it?

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @robin-dc Sorry, I thought this is your solution page. 😅

  So, if you want me to give feedback on your solution as well, give me the link to your solution. I would be happy to give some feedback on yours. 😉

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @Andrew-ezeani You're welcome! 👍

  Sorry, I don't understand your question. Are you asking for tips to make the site looks good on mobile view or on desktop view?

  Also, there are some things that I need to clarify.

  • The logo and the "Try It Free" button should live inside the header element.

  <header>
    <a href="/">
      <img src="images/logo.svg" alt="Huddle - Home" class="navigation__logo">
    </a>
    <a href="#" class="btn btn--small">
      try it free
    </a>
  </header>
  

  • For the h1 and the rest of the content should live inside the main element as the first child element of the main element.

  <main>
    <section class="hero">
       <h1 class="heading-primary">
        build the community your fans will love
      </h1>
      The content of the first section goes here...
    </section>
    the rest of the section...
  </main>
  

  • I don't recommend changing the html font size to 62.5% for mobile users (and all users). It can cause huge accessibility implications for those users with different font size or zoom requirements.I suggest reading this article by Josh Comeau where he tells about the problem of the 62.5% trick (and more!). Also, I recommend reading what an accessibility expert (Grace Snow) has said about it.
  • I notice that you use 375px for the breakpoint for the media query. So, I am assuming that you use one of the sizes on the style-guide.md. I recommend adding a media query once the site starts looking broken.

  0

  

  Robin Dela Cruz•80

  @robin-dcPosted over 2 years ago

  @vanzasetia sorry, I'm a new user 😅 https://www.frontendmentor.io/solutions/responsive-fylo-landing-page-desktopfirst-workflow-_IqAYvSQ5l This is my latest solution

  0