-
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 22 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,19 @@ | |||||
| <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; | ||||||
| } | ||||||
| .heading { | ||||||
| line-height: 0; | ||||||
| display: block; | ||||||
| top: -90px; | ||||||
| visibility: hidden; | ||||||
| } | ||||||
nathanchi marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||
| #content { | ||||||
| margin: 1rem; | ||||||
| margin-top: 75px; | ||||||
| margin-bottom: 100px; | ||||||
|
||||||
| overflow-x: hidden; | ||||||
| } | ||||||
| #logo-text { | ||||||
|
|
@@ -41,8 +52,10 @@ | |||||
| } | ||||||
| </style> | ||||||
| </head> | ||||||
| <body class="m2"> | ||||||
| <div class="fixed top-0 col-12 py1 mr3 bg-white z1"> | ||||||
| <body class=""> | ||||||
|
||||||
| <ul id="navbar" class="fixed m0 p0 overflow-hidden bg-black bottom-0 col-12 z2"> | ||||||
|
||||||
| <ul id="navbar" class="fixed m0 p0 overflow-hidden bg-black bottom-0 col-12 z2"> | |
| <ul id="navbar" class="fixed m0 p0 overflow-hidden bg-black bottom-0 col-12 z2"></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.e. on the same line)
nathanchi 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.
Why not just id: name like before? The space suffix seems odd.
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 need to use a different id for the empty space above the h2 header, that's why I'm using name-space.
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.
Would you recommend a different name?
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.
Let's pull the anonymous function here into a function called updateNavbarVisibility.
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 has been fixed.
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.
No, I didn't mean the whole thing, I meant just the arrow function.
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.
Okay, I'll fix that.
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.
So just
TOPICS.forEach(({ name }) => { const navLinkPosition = $(#${name}).position().top; $(#${name}-nav-link).text(${name} ${navLinkPosition > bottom ? '↓' : '↑'}); $(#${name}-nav-list).toggle(top >= navLinkPosition || navLinkPosition >= bottom); });?
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.
As I mentioned earlier, please use code blocks via 4 space indents.
$(window).on('scroll resize load', updateNavbarVisibility)
is what I'm looking for.
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.
Why isn't this chained?
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 had to use a different value for the li and a, so I couldn't chain it. I can remove the space, though.
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 in this entire block.