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
imgelement 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
/homepage. 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.
headershould not contain the page content. So, I suggest moving the page content inside the
mainelement. As a result, the only things that live inside the
headerare 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
buttonif you ever need to use it.
aria-labelto 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
font-size: 16pxis not necessary because by default the page is
100%width and has
16px. Also, it's not recommended to change the
:rootfont size. By doing that, you won't allow the user to change the
font-sizebased 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
Hi @vanzasetia 👋, I would love if you could review my coding techiques so I can improve more. Thank you!
@robin-dc Sorry, what do you mean by "coding techniques"?
@vanzasetia, suggestions or recommendation for my html and css codes😊
@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?
@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. 😉
@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> <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
h1and the rest of the content should live inside the
mainelement as the first child element of the
<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
htmlfont 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
375pxfor 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.
@vanzasetia sorry, I'm a new user 😅 https://www.frontendmentor.io/solutions/responsive-fylo-landing-page-desktopfirst-workflow-_IqAYvSQ5l This is my latest solution