Design comparison
Community feedback
- @stevenCsaundersPosted almost 5 years ago
@willcook4 Your solution is looking good. Just a couple of things:
1- The logo margin needs to be adjusted so it lines up with the content directly below. it is pushed in a little too far from the left.
2- I think you can get rid of one of the duplicate nav items in your HTML and just use breakpoints to style the desktop and mobile nav.
3- For the hamburger menu, you should be able to remove the label tag from the HTML. You are only using the checkbox to be able to apply styles based on the checked state in CSS.
To get rig of the google fonts HTML error, you can import directly in to CSS with "@import url('https://fonts.googleapis.com/css?family=Barlow|Barlow+Semi+Condensed&display=swap');"
Great start! A couple of tweaks and it will good to go. Hope this feedback helps,
Steve.
1@willcook4Posted almost 5 years agoThanks for the feedback Steve, always handy to get someone to look over your work.
I've fixed the logo margin (#1). Just a slip at the last minute let that through!
#2 could be done but I would need to reformat the HTML and styles. One like the example here(https://medium.com/@heyoka/responsive-pure-css-off-canvas-hamburger-menu-aebc8d11d793) would work better than the one I based mine off. Next time...
I kept the label (#3) in for accessibility, but I see now that many other menus including the example above doesn't. I think your right there. It would require me to shuffle my HTML around.
Thanks for the google font's @import trick. I've gone with the urlencoded version that turns the pipe
|
into%7C
. That should make it pass the report too. I prefer the logic of linking over an import. I'm not sure if it still applies but I have this bookmarked from reading a while ago. http://www.stevesouders.com/blog/2009/04/09/dont-use-import/ . I think even if things have improved in the browser since then, The @import flow still requires the .html file then the .css then the @import to be called. Not an issue for a little site like this but it could cascade into larger performance issues later. The link method only requires the .html then the link and that is non-blocking.Thanks for the feedback. Good to go over these things
0@stevenCsaundersPosted almost 5 years ago@willcook4 no worries. I know there are a thousand different ways to solve these problems and a lot of it is subjective.
0
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord