-
Notifications
You must be signed in to change notification settings - Fork 15
Connect Wallet not overlaps title anymore #21
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
Connect Wallet not overlaps title anymore #21
Conversation
WalkthroughThis PR removes the Footer component entirely and hides the mobile ConnectButton in the Navbar to address the layout overlap issue reported on mobile devices. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
💤 Files with no reviewable changes (1)
🔇 Additional comments (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/Navbar.jsx (1)
70-89: Remove commented dead code.The commented
navItemsarray and its references throughout the file (lines 70-89, 153-168, 261-278) should be removed entirely rather than left as comments. Keeping large blocks of commented code reduces maintainability and adds clutter.🔎 Proposed cleanup
Remove the commented
navItemsarray:- // const navItems = [ - // { - // name: "Home", - // icon: <HomeIcon className="text-current" />, - // action: () => handleScroll("home-section"), - // path: "/", - // }, - // { - // name: "Features", - // icon: <FeaturedPlayListIcon className="text-current" />, - // action: () => handleScroll("feature-section"), - // path: "/#feature-section", - // }, - // { - // name: "Services", - // icon: <DesignServicesIcon className="text-current" />, - // action: () => handleScroll("service-section"), - // path: "/#service-section", - // }, - // ]; -Also remove the commented usage blocks at lines 153-168 (desktop navigation) and lines 261-278 (mobile navigation).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/src/components/Footer.jsxfrontend/src/components/Navbar.jsxfrontend/src/page/Applayout.jsx
💤 Files with no reviewable changes (1)
- frontend/src/components/Footer.jsx
🔇 Additional comments (2)
frontend/src/page/Applayout.jsx (1)
7-16: LGTM! Footer removal is clean.The Footer component has been cleanly removed from the layout. The component now renders only the Navbar and content Outlet, which aligns with the PR objective of simplifying the layout and removing redundant code.
frontend/src/components/Navbar.jsx (1)
223-233: Fix correctly addresses the overlap issue.Commenting out the ConnectButton that was positioned next to the hamburger menu icon effectively resolves the title overlap problem on mobile. The wallet connection functionality remains accessible through the ConnectButton inside the mobile menu dropdown (lines 298-304), maintaining a good user experience.
| @@ -1,22 +0,0 @@ | |||
| import React from 'react' | |||
|
|
|||
| function Footer() { | |||
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.
We need a footer. It shouldn't be removed.
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.
|
|
||
| {/* Mobile menu button */} | ||
| <div className="md:hidden flex items-center"> | ||
| <motion.div |
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.
We need a wallet connect button. It is the most essential functionality.
The overlap shouldn't be solved simply by removing the button.
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.
OK.


The Connect Wallet button no longer overlaps the title.
The Connect Wallet button can now be accessed via the toggle menu.
Removed an unused footer that was redundant and had some logic duplicated in another file.
Closes #20
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.