-
Notifications
You must be signed in to change notification settings - Fork 2.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bose layout #2655
base: master
Are you sure you want to change the base?
Bose layout #2655
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I corrected all errors except one, I don't understand what you mean -"burger menu, must be on this page, not another" - could you explain to me? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will be fine now :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid work and definitely a good amount of effort put it here! 👍
One thing I really want to notice here: you tend to use margin-top && padding-top to position your elements. And you do it for all elements consistently. Great job!
It's a valid approach and if you started using -top
property to position elements - you should stick to it and never switch to randomly using -bottom
.
Also, you could mark your sections as rows and position them using row-gap property. but that would be just another approach. Yours is also valid and do the job.
Unfortunately, my project does not contain commits because at the end of the project I encountered a critical error that forced me to delete the entire directory and create a new one to which I moved the entire src folder.
[DEMO LINK] (https://WGoliasz.github.io/layout_miami/)