Routing Layout Changed -> Header.astro#20
Routing Layout Changed -> Header.astro#20Vishalk91-4 wants to merge 6 commits intoEshaanAgg:mainfrom
Conversation
EshaanAgg
left a comment
There was a problem hiding this comment.
This is great PR which has it's heart in the right place. But you need to shift the CSS styles to the correct components logically, instead of dumping them all in one.
|
@EshaanAgg I've updated it, so that the current layout doesn't break, could you please review it |
|
@EshaanAgg, resolved conflicts |
|
Please address the comments I have added earlier. Also attach a video of the the header showing if it is responsive or not. |
|
Resolved Comments LFX.Mentorship.and.62.more.pages.-.Work.-.Microsoft.Edge.2023-11-06.00-26-04.mp4@EshaanAgg, resolved them all |
EshaanAgg
left a comment
There was a problem hiding this comment.
Can you please explain the rationale behind changing the content of the navbar?
| path: '/org', | ||
| label: 'Organizations', | ||
| }, | ||
| { | ||
| path: '/organization/PastProjectCard', | ||
| label: 'Projects', | ||
| }, |
There was a problem hiding this comment.
These links do not exist in the site.
| {/* | ||
| const navItems = [ | ||
| { name: "Home", link: "/" }, | ||
| { name: "GitHub", link: "https://github.com/EshaanAgg/LFXMM-Frontend" }, | ||
| { name: "Team", link: "/team" }, | ||
| ]; | ||
| ]; */} |
There was a problem hiding this comment.
Why did you change the navbar content?
| </nav> | ||
| {/* | ||
| <div | ||
| class="text-2xl text-white mb-5 sm:mb-0 px-10 py-2 h-36 sm:h-20 sm:py-8 flex pb-4" |
Previously we were using ul tags, now I've changed it to navitems, routing becomes simpler
Design remains the same
@EshaanAgg, could you please review this pull request