search

utmmcss / mcss-website-frontend

MCSS Website Frontend
https://mcss.club
MIT License
0 stars 1 forks source link

Added navigation bar #7

Closed nigow closed 3 years ago

nigow commented 3 years ago

Ticket

https://trello.com/c/aekXS1Vy/5-nav-bar

What is changed / added

Added a navbar along with the MCSS logo

Screenshots

Screen Shot 2021-08-28 at 1 58 03
nigow commented 3 years ago

@TheFatPanda97 looks like i deleted a bunch of lines from package-lock.json. should i revert it?

TheFatPanda97 commented 3 years ago

@TheFatPanda97 looks like i deleted a bunch of lines from package-lock.json. should i revert it?

@nigow yep, let's discard the changes to package-lock.json

AaronCGoidel commented 3 years ago

@TheFatPanda97 looks like i deleted a bunch of lines from package-lock.json. should i revert it?

@nigow yep, let's discard the changes to package-lock.json

Since we're using npm and not yarn I think we could do with just rming the entire lock file.

TheFatPanda97 commented 3 years ago

@TheFatPanda97 looks like i deleted a bunch of lines from package-lock.json. should i revert it?

@nigow yep, let's discard the changes to package-lock.json

Since we're using npm and not yarn I think we could do with just rming the entire lock file.

I think it will be worth it to keep the lock file. The main benefit of it is so that everyone can have the exact same dependency tree. In the future, if anyone is installing the packages, they would be doing npm ci rather than npm install.

AaronCGoidel commented 3 years ago

@TheFatPanda97 looks like i deleted a bunch of lines from package-lock.json. should i revert it?

@nigow yep, let's discard the changes to package-lock.json

Since we're using npm and not yarn I think we could do with just rming the entire lock file.

I think it will be worth it to keep the lock file. The main benefit of it is so that everyone can have the exact same dependency tree. In the future, if anyone is installing the packages, they would be doing npm ci rather than npm install.

I agree; this is the more professional approach.

I thought that keeping lock files in sync (as we saw in the pr) would be too much to justify the means of staying exactly up to date.

We are meant to have a more professional dev environment in the MCSS.

TheFatPanda97 commented 3 years ago

@nigow please also fill out the PR descriptions and Trello ticket. This helps the reviewer while reviewing your PR.

nigow commented 3 years ago

Sorry, I'm slightly confused with the conversation above. So what is the verdict here? I assume it is a go with not removing package-lock.json from this change https://github.com/utmmcss/mcss-website-frontend/commit/e4f5760bc388ac3452e1d4e2b92f8a274f1e521a (hence my change is OK) but I just want to confirm first.

nigow commented 3 years ago

@AaronCGoidel regarding mobile view, I didn't really think about it. here's the mobile view: Screen Shot 2021-08-29 at 11 44 43 Would you mind merging this first and I can work on mobile view on a separate ticket? The reason is that I want to see the mobile design first. A "collapse" navbar like below would be nice, but we should agree on a design first. https://stackoverflow.com/questions/48005033/bulma-mobile-layout

TheFatPanda97 commented 3 years ago

Sorry, I'm slightly confused with the conversation above. So what is the verdict here? I assume it is a go with not removing package-lock.json from this change e4f5760 (hence my change is OK) but I just want to confirm first.

yep you are exactly right, we won't be removing package-lock.json

TheFatPanda97 commented 3 years ago

@nigow also would you mind merging master? I think there are some conflicts.

nigow commented 3 years ago

@TheFatPanda97 rebased

AaronCGoidel commented 3 years ago

view:

Alright, I'm happy saying mobile will be another ticket. This is in spec. You have my go-ahead

TheFatPanda97 commented 3 years ago

Nav bar