-
-
Notifications
You must be signed in to change notification settings - Fork 104
Feat/neon landing theme issue 141 #490
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
base: main
Are you sure you want to change the base?
Feat/neon landing theme issue 141 #490
Conversation
…hover-only, reduced-motion)\n\n- Scope cursor hiding to hover-capable, fine pointers\n- Respect prefers-reduced-motion to disable animations\n- Keep existing cursor types and trail effects intact\n\nRefs: RamakrushnaBiswal#273
…nd button - Improved hover effects on all input fields (guests, date, time) - Enhanced submit button with better scaling and translation effects - Added smooth transitions for background color changes and shadows - Maintained consistent design with amber/orange color scheme - Improved user experience with visual feedback on form interactions Refs: RamakrushnaBiswal#280
… split headline lines, prevent overlap, adjust rotated image stack\n- Global: dark page background to remove light gaps between sections\n- Navbar: transparent dark, neon hover underline, neon buttons\n- About: neon overlay and glow accents\n- Footer: neon-dark container and newsletter styling\n\nRefs: RamakrushnaBiswal#141
|
@NirajK017 is attempting to deploy a commit to the bunty's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for submitting your pull request! 🙌 We'll review it as soon as possible. In the meantime, please ensure that your changes align with our CONTRIBUTING.md. If there are any specific instructions or feedback regarding your PR, we'll provide them here. Thanks again for your contribution! 😊 |
WalkthroughIntroduces a dark-themed visual redesign across Home, Landing, Navbar, Footer, and Register pages, adds a CSS-only animated cursor system, updates hover/scroll/modal styles, and restructures several components’ markup for new layouts and effects. No public APIs or exports changed; minimal logic change adds a stable useEffect dependency in Landing. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
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: 7
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/ui/Landing.jsx (1)
15-35: Add GSAP/SplitType cleanup to prevent leaks/duplicationWithout cleanup, StrictMode double-invocation and unmounts can leak timelines and leave split DOM. Kill timeline and revert split on unmount; guard null ref.
Apply this diff:
useEffect(() => { - const splitText = new SplitType(textRef.current, { types: 'chars, words' }); - const tl = gsap.timeline({ repeat: -1, repeatDelay: 1 }); + if (!textRef.current) return; + const splitText = new SplitType(textRef.current, { types: 'chars, words' }); + const tl = gsap.timeline({ repeat: -1, repeatDelay: 1 }); // Set initial opacity to 0 for all characters gsap.set(splitText.chars, { opacity: 0 }); tl.to(splitText.chars, { opacity: 1, duration: 0.1, stagger: 0.1, ease: 'power1.inOut', }).to(splitText.chars, { opacity: 0, duration: 0.1, stagger: 0.1, ease: 'power1.inOut', delay: 1, }); -}, []); + return () => { + try { tl.kill(); } catch {} + try { splitText.revert(); } catch {} + }; +}, []);
🧹 Nitpick comments (2)
frontend/src/components/Shared/footer/Footer.jsx (1)
9-13: Mark decorative layers as hidden from assistive techAdd aria-hidden="true" to purely decorative elements to avoid noise for screen readers.
Apply this diff:
- <div className="relative h-[calc(100vh+800px)] -top-[100vh] bg-black text-white mb-4 pb-10 sm:pb-16"> - <div className="absolute inset-0 pointer-events-none"> + <div className="relative h-[calc(100vh+800px)] -top-[100vh] bg-black text-white mb-4 pb-10 sm:pb-16"> + <div className="absolute inset-0 pointer-events-none" aria-hidden="true"> <div className="absolute top-10 left-10 w-80 h-80 bg-emerald-500 rounded-full blur-3xl opacity-10"></div> <div className="absolute bottom-10 right-10 w-96 h-96 bg-green-400 rounded-full blur-3xl opacity-10"></div> </div>frontend/src/components/Shared/Navbar.jsx (1)
231-248: Harden logout flow (error handling and UX)If the logout API fails, you still clear cookies and navigate. Consider try/catch and checking response.ok; show an error if needed. Also disable the button while awaiting.
Would you like a patch to add robust error handling and a loading state?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/src/components/AnimatedCursor.css(1 hunks)frontend/src/components/Pages/Home.jsx(1 hunks)frontend/src/components/Pages/Register.jsx(1 hunks)frontend/src/components/Shared/Navbar.jsx(6 hunks)frontend/src/components/Shared/footer/Content.jsx(6 hunks)frontend/src/components/Shared/footer/Footer.jsx(1 hunks)frontend/src/components/ui/Landing.jsx(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
frontend/src/components/Shared/footer/Content.jsx (1)
frontend/src/components/Pages/Boardgame.jsx (2)
handleSubmit(30-55)
frontend/src/components/Pages/Register.jsx (1)
frontend/src/components/MainHOC.jsx (1)
MainHOC(4-18)
frontend/src/components/ui/Landing.jsx (1)
frontend/src/components/Pages/Event.jsx (1)
textRef(81-81)
🔇 Additional comments (1)
frontend/src/components/Pages/Home.jsx (1)
14-14: Home theming switch looks goodDark background + white text aligns with the new global theme.
| /* Hide default cursor only on devices with hover and fine pointer */ | ||
| @media (hover: hover) and (pointer: fine) { | ||
| * { | ||
| cursor: none !important; | ||
| } | ||
| } |
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.
Do not globally hide the native cursor by default
Hiding the cursor for all elements risks unusable UI if the custom cursor elements/logic aren’t mounted. Scope it to when the cursor is active.
Apply this diff to scope via :has (broad support) or a body class:
-/* Hide default cursor only on devices with hover and fine pointer */
-@media (hover: hover) and (pointer: fine) {
- * {
- cursor: none !important;
- }
-}
+/* Hide default cursor only when custom cursor is present */
+@media (hover: hover) and (pointer: fine) {
+ html:has(.animated-cursor) * {
+ cursor: none !important;
+ }
+}
+/* Alternatively, gate by a body class you toggle when initializing */
+/* .has-animated-cursor * { cursor: none !important; } */🤖 Prompt for AI Agents
In frontend/src/components/AnimatedCursor.css around lines 1 to 6, the
stylesheet currently hides the native cursor globally for all elements which can
break the UI if the custom cursor isn't mounted; change the selector so cursor:
none is only applied when the custom cursor is active (e.g., use a parent
selector like body.has-custom-cursor or a :has() condition that checks for the
presence of the custom cursor element) and keep the existing media query; update
any mounting/unmounting code to toggle that body class (or ensure the custom
cursor element exists to satisfy :has) so the native cursor remains visible
otherwise.
| @keyframes pulse { | ||
| 0%, 100% { | ||
| transform: scale(1); | ||
| } | ||
| 50% { | ||
| transform: scale(1.05); | ||
| } | ||
| } | ||
|
|
||
| @keyframes trailPulse { | ||
| 0%, 100% { | ||
| transform: scale(1); | ||
| opacity: 0.3; | ||
| } | ||
| 50% { | ||
| transform: scale(1.1); | ||
| opacity: 0.2; | ||
| } | ||
| } |
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.
Keyframes overwrite translate/rotate causing jumps
pulse and trailPulse set transform: scale(...) only, overwriting the base translate/rotate, which can shift the cursor. Include translate/rotate in keyframes.
Apply this diff:
@keyframes pulse {
0%, 100% {
- transform: scale(1);
+ transform: translate(-50%, -50%) rotate(45deg) scale(1);
}
50% {
- transform: scale(1.05);
+ transform: translate(-50%, -50%) rotate(45deg) scale(1.05);
}
}
@keyframes trailPulse {
0%, 100% {
- transform: scale(1);
+ transform: translate(-50%, -50%) rotate(45deg) scale(1);
opacity: 0.3;
}
50% {
- transform: scale(1.1);
+ transform: translate(-50%, -50%) rotate(45deg) scale(1.1);
opacity: 0.2;
}
}🤖 Prompt for AI Agents
In frontend/src/components/AnimatedCursor.css around lines 105 to 123, the pulse
and trailPulse keyframes only set transform: scale(...), which overwrites the
base translate/rotate and causes the cursor to jump; update each keyframe to
include the same translate(...) and rotate(...) used in the element’s base
transform (e.g. transform: translate(var(--cx), var(--cy)) rotate(var(--rot))
scale(...)) so the scale animates without removing translation/rotation, keeping
opacity changes for trailPulse as-is.
| <form className="space-y-6"> | ||
| <div className="grid grid-cols-1 md:grid-cols-2 gap-6"> |
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.
Bind submission to form onSubmit (fix Enter key and default submit)
The submit handler is on the button’s onClick. Pressing Enter won’t trigger it reliably. Move handleSubmit to form onSubmit and remove button onClick.
Apply this diff:
- <form className="space-y-6">
+ <form className="space-y-6" onSubmit={handleSubmit}>
...
- <button
+ <button
className="relative w-full py-4 px-6 text-lg font-bold text-white bg-gradient-to-r from-amber-500 via-orange-500 to-red-500 hover:from-amber-600 hover:via-orange-600 hover:to-red-600 rounded-xl shadow-lg hover:shadow-2xl transform hover:scale-[1.02] hover:-translate-y-1 transition-all duration-300 focus:outline-none focus:ring-4 focus:ring-amber-200 dark:focus:ring-amber-800 active:scale-[0.98] active:translate-y-0 overflow-hidden"
type="submit"
- onClick={handleSubmit}
>Also applies to: 213-216
🤖 Prompt for AI Agents
In frontend/src/components/Pages/Register.jsx around lines 120-121 (and
similarly at lines 213-216), the submit handler is attached to the button's
onClick which prevents form submission via Enter and default browser behavior;
move the handleSubmit prop to the <form> element as onSubmit={handleSubmit},
remove the onClick from the submit button, and ensure the handler calls
event.preventDefault() if needed and is passed the event; verify any button type
remains "submit" (or set it) so pressing Enter triggers the form onSubmit.
| <div className="p-3 bg-black/70 border border-emerald-500 rounded-md shadow-[0_0_25px_rgba(16,185,129,0.3)] w-full max-w-sm mx-auto"> | ||
| <h3 className="text-white text-base md:text-lg mb-1 md:mb-2"> | ||
| Subscribe to our Newsletter | ||
| </h3> | ||
| <form onSubmit={handleSubmit} className="flex flex-col gap-2 md:flex-row md:gap-3"> | ||
| <div className="flex items-center border rounded-md bg-white"> | ||
| <span className="p-1 md:p-2 text-gray-600">📧</span> | ||
| <div className="flex items-center border border-emerald-500 rounded-md bg-transparent"> | ||
| <span className="p-1 md:p-2 text-emerald-400">📧</span> | ||
| <input | ||
| type="email" | ||
| className="p-1 md:p-2 rounded-r-none border-l border-gray-300 focus:outline-none text-sm" | ||
| className="p-1 md:p-2 rounded-r-none border-l border-emerald-500 bg-transparent text-white placeholder-gray-400 focus:outline-none text-sm" | ||
| placeholder="Enter your email" | ||
| value={email} | ||
| onChange={(e) => setEmail(e.target.value)} | ||
| required | ||
| /> | ||
| </div> | ||
| <button type="submit" className="p-1 md:p-2 bg-green-500 text-white rounded-md"> | ||
| <button type="submit" className="p-1 md:p-2 bg-emerald-500 hover:bg-emerald-400 text-black rounded-md shadow-[0_0_18px_rgba(16,185,129,0.6)] transition"> | ||
| Subscribe | ||
| </button> | ||
| </form> | ||
| {message && <p className="mt-1 md:mt-2 text-xs md:text-sm text-green-500">{message}</p>} | ||
| {message && <p className="mt-1 md:mt-2 text-xs md:text-sm text-emerald-400">{message}</p>} | ||
| </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.
Newsletter endpoint likely incorrect/inconsistent
This component posts to /api/newsletterRouter while Boardgame.jsx uses ${VITE_BACKEND_URL}/api/newsletter/subscribe. Unify to env-based backend URL and consistent route; otherwise it breaks outside a dev proxy.
Apply this diff:
- const response = await fetch('/api/newsletterRouter', {
+ const response = await fetch(
+ `${import.meta.env.VITE_BACKEND_URL}/api/newsletter/subscribe`,
+ {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({ email }),
- });
+ }
+ );Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In frontend/src/components/Shared/footer/Content.jsx around lines 54 to 75, the
newsletter form is posting to a hardcoded/internal route that differs from
Boardgame.jsx; update the submit handler to post to the same env-driven backend
endpoint (use import.meta.env.VITE_BACKEND_URL) and the consistent route path
/api/newsletter/subscribe (construct the URL safely to avoid duplicate slashes),
replacing the current fetch/post target so the component works outside the dev
proxy.
| <h3 className="mb-2 uppercase text-white cursor-pointer">Socials</h3> | ||
| {socialLink.map((item, index) => ( | ||
| <a | ||
| target="_blank" | ||
| className="hover:text-blue-500 duration-300 flex items-center gap-2" | ||
| className="hover:text-emerald-400 duration-300 flex items-center gap-2" | ||
| key={index} |
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.
🛠️ Refactor suggestion | 🟠 Major
Add rel="noopener noreferrer" for external links
Security best practice when using target="_blank".
Apply this diff:
- <a
- target="_blank"
+ <a
+ target="_blank"
+ rel="noopener noreferrer"
className="hover:text-emerald-400 duration-300 flex items-center gap-2"
key={index}
href={item.link}
aria-label={`${item.name} - opens in a new tab`}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <h3 className="mb-2 uppercase text-white cursor-pointer">Socials</h3> | |
| {socialLink.map((item, index) => ( | |
| <a | |
| target="_blank" | |
| className="hover:text-blue-500 duration-300 flex items-center gap-2" | |
| className="hover:text-emerald-400 duration-300 flex items-center gap-2" | |
| key={index} | |
| <h3 className="mb-2 uppercase text-white cursor-pointer">Socials</h3> | |
| {socialLink.map((item, index) => ( | |
| <a | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| className="hover:text-emerald-400 duration-300 flex items-center gap-2" | |
| key={index} | |
| href={item.link} | |
| aria-label={`${item.name} - opens in a new tab`} | |
| > |
🤖 Prompt for AI Agents
In frontend/src/components/Shared/footer/Content.jsx around lines 186 to 191,
anchors using target="_blank" are missing rel attributes; update each external
link anchor to include rel="noopener noreferrer" (e.g., add rel="noopener
noreferrer" to the <a> elements that open in a new tab) to prevent opener-based
attacks and avoid exposing window.opener.
| ? 'bg-black/80 backdrop-blur-md shadow-lg text-white' | ||
| : 'bg-transparent text-white' | ||
| }`} |
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.
🛠️ Refactor suggestion | 🟠 Major
Prevent initial flash and fix token effect dependency
- Set initial isScrolled on mount to avoid a brief transparent navbar at load.
- The token sync effect (Line 29-31) runs every render; add [] to avoid redundant updates.
Apply this diff:
useEffect(() => {
- setToken(Cookies.get('authToken'));
-});
+ setToken(Cookies.get('authToken'));
+}, []);
useEffect(() => {
const handleScroll = () => {
const scrollPosition = window.scrollY;
setIsScrolled(scrollPosition > 50);
};
window.addEventListener('scroll', handleScroll);
+ // set initial state
+ handleScroll();
return () => {
window.removeEventListener('scroll', handleScroll);
};
}, []);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ? 'bg-black/80 backdrop-blur-md shadow-lg text-white' | |
| : 'bg-transparent text-white' | |
| }`} | |
| // Sync auth token once on mount | |
| useEffect(() => { | |
| setToken(Cookies.get('authToken')); | |
| }, []); | |
| // Update isScrolled on scroll and set initial state immediately | |
| useEffect(() => { | |
| const handleScroll = () => { | |
| const scrollPosition = window.scrollY; | |
| setIsScrolled(scrollPosition > 50); | |
| }; | |
| window.addEventListener('scroll', handleScroll); | |
| // set initial state | |
| handleScroll(); | |
| return () => { | |
| window.removeEventListener('scroll', handleScroll); | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
In frontend/src/components/Shared/Navbar.jsx around lines 88-90 and also
referencing lines 29-31, the navbar briefly flashes transparent on load because
isScrolled is initialized only via scroll listener, and the token sync effect is
running every render; on mount set the initial isScrolled state based on current
window.scrollY (so the class uses the correct bg immediately) and change the
token sync useEffect to include an empty dependency array ([]) so it runs only
once on mount instead of every render.
| <style jsx>{` | ||
| @import url('https://fonts.googleapis.com/css2?family=Space+Mono:wght@400;700&display=swap'); | ||
| * { | ||
| font-family: 'Space Mono', monospace; | ||
| } | ||
| `}</style> |
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.
Avoid styled-jsx and global font overrides in a Vite app
<style jsx> is a Next.js pattern; here it adds a nonstandard attribute and the wildcard selector forces Space Mono globally. Move the font import to index.html or a global CSS and scope font usage via classes or inline style on specific headings. Apply this diff to remove the block: ```diff - <style jsx>{` - @import url('https://fonts.googleapis.com/css2?family=Space+Mono:wght@400;700&display=swap'); - - * { - font-family: 'Space Mono', monospace; - } - `}</style>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **IMPORTANT**
> Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
```suggestion
🤖 Prompt for AI Agents
In frontend/src/components/ui/Landing.jsx around lines 133-139, remove the
<style jsx> block that imports Space Mono and applies a global wildcard
font-family; instead add the Google font import to the app's index.html <head>
or a global CSS file and update this component to use a scoped class or inline
style (e.g., .landing-mono or style={{ fontFamily: 'Space Mono, monospace' }})
on only the headings/elements that need the font; ensure no styled-jsx
attributes remain and that global * selectors are not used.
Summary by CodeRabbit
New Features
Style