CTASection Footer Links Unresponsive Due to Overlay Capturing Pointer Events#65
CTASection Footer Links Unresponsive Due to Overlay Capturing Pointer Events#65aniket866 wants to merge 4 commits intoStabilityNexus:mainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis pull request consolidates module imports to use path aliases, introduces new reusable hooks and constants for animations and data, reorganizes component exports through a barrel pattern, extracts hardcoded data from components, and adds new UI components including a scrollable navbar and relocated theme toggle. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/CTASection.tsx (1)
53-111: 🛠️ Refactor suggestion | 🟠 Major
SocialIconis defined but never used; inline markup duplicates it.The static analysis lint warning confirms
SocialIconis unused. The component was clearly intended to replace the repetitive inline<Link>+<Image>blocks (lines 53–111), but the refactor was left incomplete. Either use the helper or remove it.♻️ Proposed fix: use SocialIcon to eliminate duplication
Move
SocialIconaboveCTASection, then replace lines 52–112:+const SocialIcon = ({ + href, + src, + alt, +}: { + href: string; + src: string; + alt: string; +}) => ( + <Link + href={href} + target="_blank" + rel="noopener noreferrer" + className="transform transition-transform hover:scale-110" + > + <Image + src={src} + alt={alt} + width={40} + height={40} + className="h-6 w-6 object-contain md:h-10 md:w-10" + /> + </Link> +); + const CTASection = () => {Then replace the social icons block:
{/* Social Icons mapped from constants */} <div className="flex items-center gap-3 md:gap-4"> - <Link - href={SOCIAL_LINKS.website} - target="_blank" - rel="noopener noreferrer" - className="transform transition-transform hover:scale-110" - > - <Image - src="/stability.svg" - alt="Stability Nexus" - width={40} - height={40} - className="h-6 w-6 object-contain md:h-10 md:w-10" - /> - </Link> - - <Link - href={SOCIAL_LINKS.github} - ... - </Link> - - <Link - href={SOCIAL_LINKS.discord} - ... - </Link> - - <Link - href={SOCIAL_LINKS.linkedin} - ... - </Link> + <SocialIcon href={SOCIAL_LINKS.website} src="/stability.svg" alt="Stability Nexus" /> + <SocialIcon href={SOCIAL_LINKS.github} src="/socials/GitHub.svg" alt="GitHub" /> + <SocialIcon href={SOCIAL_LINKS.discord} src="/socials/Discord.svg" alt="Discord" /> + <SocialIcon href={SOCIAL_LINKS.linkedin} src="/socials/LinkedIn.svg" alt="LinkedIn" /> </div>And remove the duplicate
SocialIcondefinition at the bottom of the file (lines 119–143).Also applies to: 119-143
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/CTASection.tsx` around lines 53 - 111, The file defines a SocialIcon component but the CTASection still contains duplicated inline <Link>+<Image> blocks; replace those repeated blocks inside CTASection with usages of the SocialIcon component (e.g., <SocialIcon href={SOCIAL_LINKS.github} src="/socials/GitHub.svg" alt="GitHub" />) to DRY the code, ensure SocialIcon is declared above CTASection so it is in scope, and remove the duplicate/extra SocialIcon definition at the bottom so only the single shared component remains.
🧹 Nitpick comments (5)
components/Navbar.tsx (1)
14-45: Use semantic HTML (<header>/<nav>) for accessibility.The navbar is rendered as a
motion.divwrapping a plaindiv. Using<header>and/or<nav>improves screen-reader navigation and landmark discovery.♻️ Suggested change
- <motion.div + <motion.header className="fixed top-0 right-0 left-0 z-50 w-full bg-landing-bg dark:bg-landing-bg-dark" initial={{ y: 0 }} animate={{ y: isVisible ? 0 : -100 }} transition={{ duration: 0.3 }} > - <div className="mx-auto flex h-16 w-full max-w-[1512px] items-center justify-between px-4 md:h-[80px] md:px-[56px]"> + <nav className="mx-auto flex h-16 w-full max-w-[1512px] items-center justify-between px-4 md:h-[80px] md:px-[56px]"> ... - </div> - </motion.div> + </nav> + </motion.header>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/Navbar.tsx` around lines 14 - 45, The navbar currently uses a motion.div and a plain div which lacks semantic landmarks; update the component to use semantic elements by replacing the outer motion.div with motion.header (preserving the initial/animate/transition props and classes) and change the inner plain div that contains the logo and controls into a <nav> (or a div with role="navigation") to expose the navigation landmark; ensure the Link, Image, span, ToggleButton, and ConnectButton remain unchanged and that any className, accessibility attributes, and motion props are preserved on the new motion.header and nav elements.hooks/useTypewriter.ts (1)
40-40: Unstablewordsreference will restart the animation.The
wordsarray is in the dependency list. If the caller passes a new array reference on every render (e.g., an inline literal), the effect re-runs continuously, resetting the animation. This is fine when consuming a module-level constant likeHERO_WORDS, but worth noting for reusability — consider documenting that callers should pass a stable reference, or useuseRefinternally to track the words.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useTypewriter.ts` at line 40, The effect currently depends on the incoming words array which restarts the animation whenever callers pass a new array reference; to fix it, create a stable ref inside useTypewriter (e.g., const wordsRef = useRef(words)) and use wordsRef.current inside the effect, updating wordsRef.current only when you intentionally detect a real change (either by checking reference equality once in a separate effect or by doing a shallow/deep compare) so the effect dependency list no longer includes the unstable words reference; keep displayedText, isDeleting and wordIndex in the dependency array and document that callers should prefer a stable words array or expect the hook to detect real changes via that explicit comparison.components/cards/FeatureCard.tsx (1)
1-1: Redundant React import with thereact-jsxtransform.The
tsconfig.jsonuses"jsx": "react-jsx", which auto-injects the JSX runtime — the explicitimport React from "react"isn't needed. Not harmful, but unnecessary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/cards/FeatureCard.tsx` at line 1, Remove the redundant top-level import React from "react" in FeatureCard.tsx (the component/function FeatureCard uses JSX) because the project uses the "react-jsx" transform in tsconfig; simply delete the import line so the file relies on the automatic JSX runtime injection.components/CTASection.tsx (1)
53-66: Prefer<a>overnext/linkfor external URLs.
next/link(Link) is designed for client-side route transitions within a Next.js app. For fully external URLs (like GitHub, Discord, LinkedIn), a plain<a>tag is more semantically appropriate. WhileLinkdoes fall back to a regular anchor for external hrefs, using<a>makes the intent explicit and avoids any unnecessary prefetch behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/CTASection.tsx` around lines 53 - 66, In CTASection replace the external next/link usage: swap the Link component around the Stability Nexus icon for a plain anchor element so the external URL in SOCIAL_LINKS.website uses a semantic <a> (keep target="_blank", rel="noopener noreferrer", className="transform transition-transform hover:scale-110") and retain the existing Image component and its props (src, alt, width, height, className) inside the anchor; ensure you remove the import/usage of Link for this external link only so only internal app navigation keeps using next/link.components/ui/ToggleButton.tsx (1)
49-51: Ripple timeout is not cleaned up on unmount.If the component unmounts before the 1000ms timeout fires,
setRippleswill be called on an unmounted component. While React 18+ suppresses the warning, it's still a state leak. Consider tracking timeout IDs in a ref and clearing them in a cleanup effect, or using a self-cleaning pattern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/ToggleButton.tsx` around lines 49 - 51, The ripple removal timeout uses setTimeout(...) to call setRipples with newRipple.id but the timer ID isn't tracked or cleared on unmount, risking state updates after unmount; modify the ToggleButton component to store timeout IDs (from setTimeout) in a ref (e.g., rippleTimeoutsRef), push each timeout ID when creating a ripple in the handler that calls setRipples, and add a useEffect cleanup that clears all pending timeouts (clearTimeout for each ID) and resets the ref on unmount; ensure any removed ripple also removes its timeout from the ref to avoid leaks and reference the setRipples and newRipple identifiers when updating the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/HowItWorks.tsx`:
- Line 68: The computed progress uses (activeStep - 1) / (totalSteps - 1) which
can divide by zero if totalSteps === 1; update both places that compute
height/width for the progress bar to guard the denominator (e.g., use a
safeDenominator = Math.max(totalSteps - 1, 1) or conditional that sets progress
to 0 when totalSteps <= 1) and compute progress = ((activeStep - 1) /
safeDenominator) * 100; apply this change for the desktop progress (where height
is set) and the mobile horizontal progress (where width is set) referencing
activeStep and totalSteps in HowItWorks.tsx.
In `@components/Navbar.tsx`:
- Line 17: The Navbar visually hides via animate={{ y: isVisible ? 0 : -100 }}
but remains focusable; update the Navbar component to also programmatically
remove the element from accessibility and tab order when hidden: add a ref to
the navbar container (e.g., navbarRef), set aria-hidden={!isVisible} on that
container, and in a useEffect toggle the inert attribute (or
setAttribute('inert','true') / removeAttribute('inert')) when isVisible changes
so child links/buttons become unfocusable; alternatively, after the hide
transition completes (handle onAnimationComplete or onTransitionEnd) set
visibility: hidden and inert to fully remove from tab order, and ensure you
reference isVisible, the animate prop, and the navbar container/ref in your
changes.
In `@components/ui/ToggleButton.tsx`:
- Around line 6-15: ToggleButton currently performs client-only reads
(localStorage/matchMedia) during state initialization causing an SSR hydration
mismatch; change to a deterministic server-safe init (e.g., const [isDarkMode,
setIsDarkMode] = useState<boolean>(false)) and move the actual theme detection
into a useEffect in ToggleButton that runs on mount to call setIsDarkMode (or
integrate next-themes ThemeProvider at the app root and consume its hook
instead); render a stable placeholder (no theme-dependent DOM) until the effect
resolves to avoid mismatches. Also fix the ripple setTimeout cleanup in
ToggleButton by storing the timer id in a ref and clearing it in a useEffect
cleanup (and guard any setIsDarkMode/state updates by checking mounted state) to
prevent updates after unmount.
In `@hooks/useTypewriter.ts`:
- Around line 7-14: The hook useTypewriter reads words[wordIndex] into
currentWord without handling an empty words array, which will make currentWord
undefined and cause a TypeError when accessing currentWord.length; fix it by
guarding early inside useEffect (or at the top of the hook) to return/do nothing
if words.length === 0 or currentWord is falsy, e.g., check words.length and/or
if (!currentWord) before using currentWord in the timer logic and ensure any
timers are cleared when returning; update references to currentWord, wordIndex,
and the useEffect cleanup accordingly.
---
Outside diff comments:
In `@components/CTASection.tsx`:
- Around line 53-111: The file defines a SocialIcon component but the CTASection
still contains duplicated inline <Link>+<Image> blocks; replace those repeated
blocks inside CTASection with usages of the SocialIcon component (e.g.,
<SocialIcon href={SOCIAL_LINKS.github} src="/socials/GitHub.svg" alt="GitHub"
/>) to DRY the code, ensure SocialIcon is declared above CTASection so it is in
scope, and remove the duplicate/extra SocialIcon definition at the bottom so
only the single shared component remains.
---
Nitpick comments:
In `@components/cards/FeatureCard.tsx`:
- Line 1: Remove the redundant top-level import React from "react" in
FeatureCard.tsx (the component/function FeatureCard uses JSX) because the
project uses the "react-jsx" transform in tsconfig; simply delete the import
line so the file relies on the automatic JSX runtime injection.
In `@components/CTASection.tsx`:
- Around line 53-66: In CTASection replace the external next/link usage: swap
the Link component around the Stability Nexus icon for a plain anchor element so
the external URL in SOCIAL_LINKS.website uses a semantic <a> (keep
target="_blank", rel="noopener noreferrer", className="transform
transition-transform hover:scale-110") and retain the existing Image component
and its props (src, alt, width, height, className) inside the anchor; ensure you
remove the import/usage of Link for this external link only so only internal app
navigation keeps using next/link.
In `@components/Navbar.tsx`:
- Around line 14-45: The navbar currently uses a motion.div and a plain div
which lacks semantic landmarks; update the component to use semantic elements by
replacing the outer motion.div with motion.header (preserving the
initial/animate/transition props and classes) and change the inner plain div
that contains the logo and controls into a <nav> (or a div with
role="navigation") to expose the navigation landmark; ensure the Link, Image,
span, ToggleButton, and ConnectButton remain unchanged and that any className,
accessibility attributes, and motion props are preserved on the new
motion.header and nav elements.
In `@components/ui/ToggleButton.tsx`:
- Around line 49-51: The ripple removal timeout uses setTimeout(...) to call
setRipples with newRipple.id but the timer ID isn't tracked or cleared on
unmount, risking state updates after unmount; modify the ToggleButton component
to store timeout IDs (from setTimeout) in a ref (e.g., rippleTimeoutsRef), push
each timeout ID when creating a ripple in the handler that calls setRipples, and
add a useEffect cleanup that clears all pending timeouts (clearTimeout for each
ID) and resets the ref on unmount; ensure any removed ripple also removes its
timeout from the ref to avoid leaks and reference the setRipples and newRipple
identifiers when updating the logic.
In `@hooks/useTypewriter.ts`:
- Line 40: The effect currently depends on the incoming words array which
restarts the animation whenever callers pass a new array reference; to fix it,
create a stable ref inside useTypewriter (e.g., const wordsRef = useRef(words))
and use wordsRef.current inside the effect, updating wordsRef.current only when
you intentionally detect a real change (either by checking reference equality
once in a separate effect or by doing a shallow/deep compare) so the effect
dependency list no longer includes the unstable words reference; keep
displayedText, isDeleting and wordIndex in the dependency array and document
that callers should prefer a stable words array or expect the hook to detect
real changes via that explicit comparison.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Addressed Issues:
#55
Screenshots/Recordings:
screen-capture.34.webm
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
@KanishkSogani Please have a look
Summary by CodeRabbit
Release Notes
New Features
Chores