-
Notifications
You must be signed in to change notification settings - Fork 6
added fixed navbar #46
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
base: master
Are you sure you want to change the base?
Changes from 14 commits
748a10f
1a44b6b
e9595a6
faf010d
dd265b8
e778e69
4103dfb
ab7d0a9
c6fb817
e9e36e5
80d0351
4a8f35f
a9e5cfe
6377cb9
6aed63a
71b2028
a2c8ce9
53fbcf4
d1527a3
920281d
897728e
8c7f959
98f71a1
c5a4337
4f1502a
7bafb7f
773d925
3e5f3d3
d1d50a9
764b4b6
05c0894
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -10,8 +10,16 @@ | |||||||||
| <link rel="stylesheet" href="https://unpkg.com/basscss@7.1.1/css/basscss.min.css" integrity="sha384-/biuPsPEkt10QoeOExIADuWNrpFFXALKPgBhEDkRdKDJdQOPpbNU3uUeFbL/KPJg" crossorigin="anonymous"> | ||||||||||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/hint.css/2.5.0/hint.base.min.css" integrity="sha256-ODFgZG+Y3V10lzZu+7J8epM4SdW0w3p8qbVmmShNrtk=" crossorigin="anonymous"> | ||||||||||
| <style type="text/css"> | ||||||||||
| #navbar li a { | ||||||||||
| padding: 14px 16px; | ||||||||||
| } | ||||||||||
| .topic { | ||||||||||
| padding-top: 75px; | ||||||||||
| margin-top: -75px; | ||||||||||
| } | ||||||||||
| #content { | ||||||||||
| margin-top: 75px; | ||||||||||
| margin-bottom: 100px; | ||||||||||
| overflow-x: hidden; | ||||||||||
| } | ||||||||||
| #logo-text { | ||||||||||
|
|
@@ -42,6 +50,9 @@ | |||||||||
| </style> | ||||||||||
| </head> | ||||||||||
| <body class="m2"> | ||||||||||
| <ul class="fixed m0 p0 overflow-hidden bg-black bottom-0 col-12 z2" id="navbar"> | ||||||||||
|
||||||||||
| <ul class="fixed m0 p0 overflow-hidden bg-black bottom-0 col-12 z2" id="navbar"> | |
| <ul id="navbar" class="fixed m0 p0 overflow-hidden bg-black bottom-0 col-12 z2"></ul> |
Outdated
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.
What does this li class do?
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.
This is the hardcoded li class that I added. The goes within the <li>, which goes within the <ul>.
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.
It's used for the .append() method later on in the code, to select only those values.
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.
Why not just #navbar li?.
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.
Unresolved?
sushain97 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
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.
| $(window).on('scroll resize', function(){ | |
| $(window).on('scroll resize', () => { |
Outdated
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.
The rest of the code is indented two spaces but now it's 4?
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'll run it through Prettier again, then.
Outdated
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.
| TOPICS.forEach(topic => { | |
| TOPICS.forEach(({name}) => { |
Outdated
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.
| const name_position = $(`#${name}`).position().top; | |
| const navLinkPosition = $(`#${name}`).position().top; |
Outdated
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.
Can replace the entire block with something like:
| $(`#${name}-nav-link`).text(`${name} ${name_position > bottom ? '↓' : '↑'}`).show(); | |
| $(`#${name}-nav-link`) | |
| .text(`${name} ${name_position > bottom ? '↓' : '↑'}`) | |
| .toggle(top >= name_position || name_position >= bottom); |
Outdated
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.
Indentation is off here.
Outdated
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.
This entire block can be replaced with $('.li').append(TOPICS.map(({name}) => ...
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.
Also, why are you appending all the links to the same list element? There should be one list element (<li>) per entry inside the <ul>.
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 tried replacing TOPICS.forEach(topic => { as you recommended, but it didn't work.
I also tried to make one list element (each containing one / element), but it didn't quite work. I tried doing this:
TOPICS.map(({name}) => { $('#navbar').append( $('<li></li>', {class:${name} left}) $('#navbar li').append( $('<a class = "inline-block white center btn:hover">', { id: ${name}-nav-link, href: #${name} }).text(name) ); ); });, which failed. Do you have any suggestions?
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.
Nevermind, it works now.
Outdated
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.
| $('<a class = "inline-block white center btn:hover">', { | |
| $('<a class="inline-block white center btn:hover">', { |
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.
Instead of using 3 margin rules, just use one with 4 values.