@vanzasetia
Posted
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
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 themainelement. As a result, the only things that live inside theheaderare 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
typeof thebuttonif you ever need to use it. - Add
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
htmlelement. Settingmax-width: 100vwandfont-size: 16pxis not necessary because by default the page is100%width and has16px. Also, it's not recommended to change thehtmlor:rootfont size. By doing that, you won't allow the user to change thefont-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
1
@robin-dc
Posted
Hi @vanzasetia 👋, I would love if you could review my coding techiques so I can improve more. Thank you!
0
@vanzasetia
Posted
@robin-dc Sorry, what do you mean by "coding techniques"?
0
@robin-dc
Posted
@vanzasetia, suggestions or recommendation for my html and css codes😊
0
@Andrew-ezeani
Posted
@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
@vanzasetia
Posted
@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
@vanzasetia
Posted
@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
headerelement.
<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 themainelement as the first child element of themainelement.
<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 to62.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 thestyle-guide.md. I recommend adding a media query once the site starts looking broken.
0
@robin-dc
Posted
@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
