@stevenCsaunders
Steven Saunders
@stevenCsaundersAll comments
- @stevenCsaunders
@mattstuddert I made the suggested changes and went with the ITCSS structure. Just looking for some feedback on the structure of the files as well as the cleaned up SASS.
Thanks!
- @WalrusPrince16@stevenCsaunders
@walrusprince16 Great job on the solution!
To ease into the active state you can add a CSS transition on the opacity property on your container for the mobile nav.
One other thing to think about is sticking to only classes for CSS selectors. Use IDs if necessary for Javascript. IDs add unnecessary specificity to the HTML/CSS and cannot be reused. Classes can be used multiple times and in case you need additional specificity in your CSS you can add more than one class to an element.
- @willcook4@stevenCsaunders
@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.
