-
-
Notifications
You must be signed in to change notification settings - Fork 771
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
gpg/gc-menu-nav #8797
gpg/gc-menu-nav #8797
Conversation
…he same nav to the wiki
…h of the "What's Possible" menu
app/assets/v2/js/shared.js
Outdated
const breakpoint_sm = 576; | ||
const breakpoint_md = 768; | ||
const breakpoint_lg = 992; | ||
const breakpoint_xl = 1200; |
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.
Maybe you want to get it from the css variables
getComputedStyle(document.documentElement).getPropertyValue('--breakpoint-sm');
--breakpoint-xs: 0;
--breakpoint-sm: 576px;
--breakpoint-md: 768px;
--breakpoint-lg: 992px;
--breakpoint-xl: 1200px;
But just a suggestion as probably could delay the nav load to be ready
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.
Good idea 👍 I'm already reading --gc-menu-transition-duration
from the styles so it shouldn't introduce any further delay...
I've bound the whole of navbar.js
to DOMContentLoaded
and added showMenu()
to the click
event - those changes are now on staging, it seems to work well but let me know if you think the delay is too much and I can switch it back. 7cd0d2e
<!-- Main --> | ||
{% block 'main' %} | ||
{% compress css file receive_bulk %} | ||
<link rel="stylesheet" type="text/x-scss" href={% static "v2/scss/jquery-ui.scss" %} /> |
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.
😢
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 great,
just an small thing, seems there are some regressions on https://stage.gitcoin.co/tip with the profile image and as the nav doesn't exist there
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.
nice work man... this is 🔥
only replace URL paths with django URL tag... i pointed some but you can check others i may have missed
I've resolved this (f454427) and updated the |
Description
This PR implements the new menu designs using an alternative approach (compared to #8756)
PRD: https://docs.google.com/...F7Cz_Fa0nAkMR3Yef1-eyU5c
Figma: https://www.figma.com/...Design-System---WIP-020221
Tasks from the PRD:
Keeps the “Products” drop down as is.In addition, this PR also:
height:auto
so that there is only one scrollable area in the mobile navRefers/Fixes
Refers: #8756
Closes: #8367
Testing
This branch is live on the staging server 🚀
Desktop view
Mobile view