Code review for my header / nav please
hi, i am getting into web dev and responsive design and mobile menus have been my bane, making an effort to get better and I was hoping someone could look over my project structure and see if this is a good template to build off of or if its too verbose, as well as look at my styling usage and critique as they see fit!
repo: https://github.com/griffnsh/web-starter
GitHub
GitHub - griffnsh/web-starter
Contribute to griffnsh/web-starter development by creating an account on GitHub.
20 Replies
uhh i think the hamburger should appear when it reaches the breakpoint of mobile view
you could shrink the size of the logo and font size of the nav items (a tiny bit) tho
Quick question: did you use JavaScript to make that drop down menu?
Navigation menus are really, really tricky so don't get discouraged. I've been coding for a few years now and I still have trouble with them 😂
that being said, Kevin has some good videos on responsive nav menus. I don't have them off the top of my head but he goes over some simple examples and some really light accessibility features with them too
But I think for now if you're new, just focus on getting it working, then you can focus on it being more maintainable to make it easier for you to add / edit stuff in the future. But right now I think what you have is easy to read and maintainable! Just lacking in accessibility but don't worry about that for now (unless you want to ofc but it's another rabbit hole)
Yes but just regular DOM manipulation
Big thanks I appreciate you taking the time to look it over. Definitely wanna work towards accessibility and eventually making my own library as I actually upskill towards making a valuable one lol
As far as my sass structure and the way I am using @use would you say this is decent practice or veering towards bad practice?
hm let me check
oh nice is this the 7-1 architecture?
Ehhh sorta
It’s like that Chex mix saying I just took the parts I liked and left the rest
For projects that go at scale I’d go full 7-1
that's a good approach. no need to abstract too heavily. we've all been there lol
Here's a project I did around a year ago and this is how I'd probably build it out again in terms of styling https://github.com/v11ncent/fun-repo
So if you go here: https://github.com/v11ncent/fun-repo/tree/master/src/styles, you'll see I abstracted all my reusable variables (tokens) into their own file
And then I have a master tokens file: https://github.com/v11ncent/fun-repo/blob/master/src/styles/tokens/_tokens.scss that imports all of them. This means I can go in and import the master token file inside other sass files using something like
@use "../styles/tokens/tokens" as t;
Anyway I'm kind of rambling on here but I agree with your approach just wanted to show you if you have a more complete website you can maybe look into doing the 7-1 and mixing in some of what I have for inspiration. I started with the 7-1 and kind of developed my own workflow that I liked which is in that repoRambling much appreciated I’m gonna let this marinate and if I have any questions I’ll holler thanks again
sounds good! lmk
Looked thru and I really like the structure and thought it was awesome you use Astro without tailwind it’s been annoying trying to find Astro projects without tailwind or being in other frameworks so that was also cool to poke around and see
Im a little ways away from being proficient with Astro but that’s my goal for sure
Ye you don't need to use astro either I just like it cause it let's you easily make components and use markdown to make blog posts
My philosophy is trying to keep projects as "lean" as possible -- don't use unnecessary libraries or frameworks
@vince hey in your fun repo for
is this just making it so you can do './' or '/' to the images without having to path around?
Nah so iirc that's Astro's Image component. Basically it allows me to optimize images at build time. For example, because I'm throwing in my image as an argument to that component, Astro will automatically compress it / size it correctly / convert to webp (depending on if I have that set up) when I do
npm run build, rather than me having to do that all manuallyhey was looking into your project structure more while I'm digesting what I've been learning with astro and was curious how your SCSS to CSS output is setup since the package.json doesn't have node-sass as a dev dependency as well
or actually nvm i see Sass at the bottom just confused about an output script my bad
no worries, I couldn't tell ya anyways lol. i think i just followed a guide on setting up sass with astro lol
it might come out of the box i don't remember tbh
word, it honestly might just be straight out of the box since the only mention of Sass I see in the docs is to just npm i it lol
I think it is, you should use it anyway 😉 very useful (def not biased)
hahaha im not too engaged in the CSS wars, im not a tailwind guy myself so I really love SCSS
and yeah I just added to my index.astro code fence and it seemed to automatically handle it which is awesome if im not having to worry about output
@vince hate to pester, curious what the -* is applying when you forward these tokens from your fun-repo
no worries keep pestering! so that's something that took me months to figure out how to do lol. It lets you "forward" partials / styles in scss.
So basically I can import all of those tokens into a stylesheet with:
and then from there I can use any of those variables I defined by using their prefix. for example, if I wanted to use my main font-family, I can do this