Memory section#8
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Deploy Preview for devhack3 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “Memories” hero/gallery experience and updates several sections/interactions across the site (Themes, Team, Timeline, navbar cursor targeting).
Changes:
- Add a photo dataset and replace the hero’s horizontal marquee rows with a vertical scrolling gallery + “Memories” title animation.
- Redesign the Team section into tabbed grids with a bio modal, and update Timeline to use a submarine icon on mobile.
- Add a custom target-following cursor, new CSS utilities/animations, and adjust Themes to use inline expand/collapse instead of a modal.
Reviewed changes
Copilot reviewed 14 out of 44 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/sections/photos.ts | Adds a shared photo list used by the hero gallery. |
| src/sections/Hero.tsx | Reworks hero into vertical scrolling gallery + “Memories” title and GSAP scroll animation. |
| src/sections/Team.tsx | Replaces carousel-based team UI with tabbed grids + bio modal. |
| src/sections/Timeline.tsx | Adds a mobile submarine SVG and updates smoke/bubble animation behavior on mobile. |
| src/components/Themes.tsx | Replaces modal-based theme details with inline hover/tap expansion and adds mobile detection hook. |
| src/components/BlurText.tsx | Introduces animated blur-in text component used by the hero title. |
| src/components/TargetCursor.tsx | Adds custom cursor behavior that targets elements with .cursor-target. |
| src/components/TargetCursor.css | Styles for the custom cursor. |
| src/index.css | Adds font import + CSS utilities and new keyframes for vertical scroll + coin spin. |
| src/App.tsx | Enables Team section and mounts TargetCursor; adds .cursor-target to navbar items. |
| src/components/TeamCarousel.tsx | Removes the old carousel component (no longer referenced). |
| package.json | Adds react-photo-album dependency. |
| package-lock.json | Lockfile updates for the new dependency and dependency graph changes. |
| CHANGELOG.md | Documents section and interaction changes introduced in this PR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button | ||
| className="absolute top-3 right-3 sm:top-4 sm:right-4 w-8 h-8 sm:w-10 sm:h-10 border-2 border-[#1a1a1a] flex items-center justify-center font-bold font-mono hover:bg-[#1a1a1a] hover:text-[#f3ecd2] transition-colors text-sm sm:text-base" | ||
| onClick={() => setSelectedBio(null)} | ||
| > | ||
| X | ||
| </button> |
There was a problem hiding this comment.
The modal close button only shows "X" and has no accessible label. Add an aria-label (e.g., "Close bio") so screen readers announce the action clearly.
| "react": "^19.2.4", | ||
| "react-dom": "^19.2.4", | ||
| "react-icons": "^5.6.0", | ||
| "react-photo-album": "^3.6.0", |
There was a problem hiding this comment.
react-photo-album was added to dependencies but isn’t imported/used anywhere in the current PR. If it’s not needed yet, consider removing it to avoid extra install surface and potential bundle size impact; if it is needed, add the corresponding usage so the dependency is justified.
| "react-photo-album": "^3.6.0", |
| if (selectedBio) { | ||
| if (typeof window !== 'undefined') document.body.style.overflow = 'hidden'; | ||
| } else { | ||
| if (typeof window !== 'undefined') document.body.style.overflow = 'auto'; | ||
| } |
There was a problem hiding this comment.
Avoid mutating document.body.style.overflow during render. This runs on every render and can leave the page in a bad state if the component unmounts while the modal is open. Move this into a useEffect that depends on selectedBio, and restore the previous overflow value in the cleanup.
| { id: '1', name: 'Trisha', role: 'Student Co ordinator', image: 'https://api.dicebear.com/9.x/pixel-art/svg?seed=Trisha' }, | ||
| { id: '2', name: 'S Shreenidhi', role: 'Student Co ordinator', image: 'https://api.dicebear.com/9.x/pixel-art/svg?seed=Shreenidhi' }, |
There was a problem hiding this comment.
User-facing role text has a spelling issue: "Student Co ordinator" should be "Student Coordinator" (or "Co-ordinator", but consistently). Fixing this avoids showing the typo in the UI.
| <div className="flex gap-3 sm:gap-4"> | ||
| <a | ||
| href={selectedBio.twitter || '#'} | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| className="flex-1 py-2.5 sm:py-3 border-2 border-[#1a1a1a] text-center font-bold uppercase tracking-wider sm:tracking-widest hover:bg-[#1a1a1a] hover:text-[#f3ecd2] transition-colors text-xs sm:text-sm md:text-base" | ||
| > | ||
| Twitter/X | ||
| </a> | ||
| <a | ||
| href={selectedBio.linkedin || '#'} | ||
| target="_blank" | ||
| rel="noreferrer" | ||
| className="flex-1 py-2.5 sm:py-3 border-2 border-[#1a1a1a] text-center font-bold uppercase tracking-wider sm:tracking-widest hover:bg-[#1a1a1a] hover:text-[#f3ecd2] transition-colors text-xs sm:text-sm md:text-base" |
There was a problem hiding this comment.
The social links fall back to href="#" when a URL isn’t present. Clicking these will still navigate (jump to top) and opens a blank tab due to target="_blank". Prefer conditionally rendering the link only when a URL exists, or disable the button-style link when missing.
| // On desktop: hover controls reveal; on mobile: tap toggles | ||
| const showContent = isMobile ? isOpen : isOpen; | ||
|
|
There was a problem hiding this comment.
const showContent = isMobile ? isOpen : isOpen; is equivalent to just isOpen and the comment implies different desktop vs mobile behavior. Simplifying this (or using separate hover/tap state if intended) will make the intent clearer.
| initialize(); | ||
|
|
||
| return () => { | ||
| }; | ||
| }, []); |
There was a problem hiding this comment.
initialize() sets up GSAP context + a resize listener and returns a cleanup function, but the effect doesn’t use that return value (the useEffect cleanup is empty). This leaks the resize handler and ScrollTrigger/GSAP context on unmount/remount. Capture/return the cleanup from initialize (or inline setup in the effect and return context.revert() + listener removal).
| const Column = ({ reverse = false, speed = 40, offset = false, className = "", photoSubset }: { reverse?: boolean; speed?: number; offset?: boolean; className?: string; photoSubset: typeof photos }) => { | ||
| const colPhotos = [...photoSubset, ...photoSubset]; | ||
|
|
||
| return ( | ||
| <div className="overflow-hidden w-full"> | ||
| <div className={`h-full overflow-hidden w-full relative ${className}`}> | ||
| <div | ||
| className={`flex gap-1 sm:gap-3 md:gap-4 w-max ${reverse ? "hero-scroll-row-reverse" : "hero-scroll-row" | ||
| } ${speedClass}`} | ||
| className={`flex flex-col gap-5 w-full ${reverse ? 'animate-scroll-y-reverse' : 'animate-scroll-y'}`} | ||
| style={{ animationDuration: `${speed}s`, paddingTop: offset ? '120px' : '0' }} | ||
| > | ||
| {[...images, ...images].map((src, i) => { | ||
| const sizeVariants = [ | ||
| "h-[120px] sm:h-[110px] md:h-[140px] lg:h-[170px]", | ||
| "h-[140px] sm:h-[130px] md:h-[160px] lg:h-[200px]", | ||
| "h-[130px] sm:h-[120px] md:h-[150px] lg:h-[180px]", | ||
| ]; | ||
|
|
||
| const randomSize = sizeVariants[i % sizeVariants.length]; | ||
|
|
||
| return ( | ||
| <img | ||
| key={i} | ||
| src={src} | ||
| alt="" | ||
| className={`${randomSize} w-auto object-cover rounded-md sm:rounded-lg shrink-0`} | ||
| /> | ||
| ); | ||
| })} | ||
| {[1, 2].map(key => ( | ||
| <div key={key} className="flex flex-col gap-5 w-full px-[10px]"> | ||
| {colPhotos.map((p, i) => ( |
There was a problem hiding this comment.
Column duplicates the photo list twice (colPhotos = [...photoSubset, ...photoSubset]) and then renders it twice again via {[1, 2].map(...)}. That produces 4× the DOM nodes per column (hundreds of <img> elements across 4 columns), which is likely to hurt layout/paint performance. Consider duplicating only once (either duplicate the array OR render it twice) and ensure keys remain stable across the duplicated segments.
| /** Reusable section sub-heading */ | ||
| function SectionSubHeading({ children }: { children: React.ReactNode }) { | ||
| return ( | ||
| <h3 className="text-2xl sm:text-3xl lg:text-5xl font-black uppercase tracking-widest mb-6 sm:mb-8 md:mb-12 text-center w-full"> | ||
| {children} | ||
| </h3> | ||
| ); | ||
| } |
There was a problem hiding this comment.
This file uses React.ReactNode / React.CSSProperties but doesn’t import the React namespace (only useState). With tsconfig set to jsx: react-jsx, React isn’t globally available, so this will fail type-checking. Import type ReactNode/CSSProperties from react, or import type React from 'react', and update the annotations accordingly.
| <button | ||
| key={tab} | ||
| onClick={() => onChange(tab)} | ||
| className="relative px-5 py-2 rounded-full text-sm sm:text-base font-bold uppercase tracking-wider transition-all duration-300 border-2 cursor-pointer" | ||
| style={{ | ||
| borderColor: isActive ? accent : '#1a1a1a', | ||
| backgroundColor: isActive ? accent : 'transparent', | ||
| color: isActive ? '#fff' : '#1a1a1a', | ||
| boxShadow: isActive ? `0 4px 16px ${accent}40` : 'none', | ||
| }} | ||
| onClick={() => onClickBio(member)} | ||
| className="text-[11px] sm:text-xs md:text-sm font-bold uppercase tracking-widest hover:text-[#f97028] transition-colors flex items-center gap-1.5 sm:gap-2" | ||
| > | ||
| {tab} | ||
| <span className="text-[#f97028]">+</span> SHOW BIO | ||
| </button> |
There was a problem hiding this comment.
TeamCard already calls onClickBio(member) on the whole card (onClick on the wrapping motion.div). The inner “SHOW BIO” button also calls onClickBio, so clicking the button will trigger the handler twice via event bubbling. Remove the button’s onClick or stop propagation to avoid duplicate state updates.
Don't merge automatically, it has conflicts, just use the sections