Fix issue #50 Make navbar functional - #51
Conversation
modified: src/sections/CallToAction.tsx modified: src/sections/Features.tsx modified: src/sections/Header.tsx
📝 WalkthroughWalkthroughAdded stable DOM ids to Features and CallToAction sections, a smooth-scrolling click handler wired in Header for in-page navigation, interactive hover/active styling on Button, and a LogoTicker update that adds logos and adjusts looping/animation timing. No exported/public API changes. Changes
Sequence Diagram(s)(Skipped — changes are small UI additions and do not introduce complex multi-component control flows warranting a sequence diagram.) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/sections/Header.tsx (2)
25-30: Consider extracting the id from href to reduce duplication.The current implementation passes the id both in the
hrefattribute and as a parameter tohandleScroll. You could extract it from the href instead.🔎 Proposed refactor to eliminate duplication
- const handleScroll = (e: React.MouseEvent<HTMLAnchorElement>, id: string) => { + const handleScroll = (e: React.MouseEvent<HTMLAnchorElement>) => { e.preventDefault() + const id = e.currentTarget.getAttribute('href')?.slice(1) + if (!id) return const element = document.getElementById(id) if (element) { element.scrollIntoView({ behavior: 'smooth', block: 'start' }) } }Then update the links:
- <a href="#features" onClick={(e) => handleScroll(e, 'features')} className="text-white/70 hover:text-white transition"> + <a href="#features" onClick={handleScroll} className="text-white/70 hover:text-white transition"> Features </a> - <a href="#developer-guide" onClick={(e) => handleScroll(e, 'developer-guide')} className="text-white/70 hover:text-white transition"> + <a href="#developer-guide" onClick={handleScroll} className="text-white/70 hover:text-white transition"> Developers Guide </a>
31-33: Consider updating the "Integration Docs" link for consistency.The "Integration Docs" link still uses
href="#"without a scroll handler, which is inconsistent with the updated "Features" and "Developers Guide" links. If this link should navigate elsewhere or remain as a placeholder, consider adding a comment or handling it consistently.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/sections/CallToAction.tsxsrc/sections/Features.tsxsrc/sections/Header.tsx
🔇 Additional comments (4)
src/sections/CallToAction.tsx (1)
49-49: LGTM! Scroll anchor added correctly.The
id="developer-guide"attribute establishes the scroll target for the navbar's "Developers Guide" link.src/sections/Features.tsx (1)
133-133: LGTM! Scroll anchor added correctly.The
id="features"attribute establishes the scroll target for the navbar's "Features" link.src/sections/Header.tsx (2)
1-1: Correct use of 'use client' directive.The directive is necessary for client-side event handlers and DOM manipulation in the scroll handler.
7-13: LGTM! Smooth scroll implementation is functional.The
handleScrollfunction correctly prevents default anchor behavior and implements smooth scrolling with proper null checking.
modified: src/components/Button.tsx
new file: src/assets/StabilityNexus.png new file: src/assets/StabilityNexus.svg new file: src/assets/StablePay.svg new file: src/assets/djed-alliance.png modified: src/sections/LogoTicker.tsx
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/sections/LogoTicker.tsx (2)
9-12: Simplify the items array to avoid redundancy.The
itemsarray currently contains duplicates (Stability Nexus and Djed Alliance each appear twice), and then the array is doubled again on line 35 with[...items, ...items]. This results in 8 rendered items when only 2 unique logos exist.🔎 Proposed refactor to define only unique items
const items = [ { label: 'Stability Nexus', logo: StabilityNexusLogo }, { label: 'Djed Alliance', logo: DjedAllianceLogo }, - { label: 'Stability Nexus', logo: StabilityNexusLogo }, - { label: 'Djed Alliance', logo: DjedAllianceLogo }, ]The doubling logic on line 35 will handle the repetition needed for seamless looping.
35-40: Consider more explicit keys for duplicate items.While using
indexas the key is acceptable for this static content, a more explicit key likekey={\${item.label}-${index}`}` would make the relationship clearer, especially given that the same logos appear multiple times.🔎 Optional improvement for key prop
- {[...items, ...items].map((item, index) => ( - <div key={index} className="flex items-center gap-2"> + {[...items, ...items].map((item, index) => ( + <div key={`${item.label}-${index}`} className="flex items-center gap-2"> <Image src={item.logo} alt={item.label} className="h-5 w-5 sm:h-6 sm:w-6 object-contain" width={24} height={24} />
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
src/assets/StabilityNexus.pngis excluded by!**/*.pngsrc/assets/StabilityNexus.svgis excluded by!**/*.svgsrc/assets/StablePay.svgis excluded by!**/*.svgsrc/assets/djed-alliance.pngis excluded by!**/*.png
📒 Files selected for processing (1)
src/sections/LogoTicker.tsx
🔇 Additional comments (2)
src/sections/LogoTicker.tsx (2)
3-5: LGTM! Appropriate use of Next.js Image component.The imports are correct, and using the Next.js
Imagecomponent will provide automatic optimization for the logos.
27-27: Animation logic correctly implements seamless looping.The
-50%translation combined with the doubled items array (line 35) creates a smooth infinite loop: when the animation completes, the visual position matches the starting state. The reduced duration speeds up the ticker appropriately.Also applies to: 30-30
1 participant
Fixes #50 and Fixes #49 and Fixes #4
Summary by CodeRabbit
New Features
Style
✏️ Tip: You can customize this high-level summary in your review settings.