Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis pull request refactors the partners page from a client component to a server component, extracts photo gallery logic into a dedicated PhotoGallery component, adds interactive drag-based scrolling to timeline controls, migrates CTA buttons from anchor tags to Next.js Link components, and converts internal navigation from external URLs to relative routes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
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: 4
🧹 Nitpick comments (6)
app/for-partners/PhotoGallery.tsx (2)
109-141: Consider using CSS-based pagination instead of hiding items withhiddenclass.All 10 photos are rendered and hidden conditionally via the
hiddenclass. For larger galleries, this could affect performance. Consider slicing the array to only render visible items:♻️ Proposed refactor
<div className="grid grid-cols-1 md:grid-cols-2 lg:grid-cols-3 gap-6"> - {eventPhotos.map((photo, index) => { - const isVisible = index >= photoIndex && index < photoIndex + 6 + {eventPhotos.slice(photoIndex, photoIndex + 6).map((photo) => { - ... - className={`...${!isVisible ? ' hidden' : ''}`} + ... + className={`...`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/for-partners/PhotoGallery.tsx` around lines 109 - 141, The gallery currently maps over the entire eventPhotos array and toggles visibility with the hidden class (using photoIndex), which renders all items and harms performance; change the rendering to only map the visible subset by computing a slice like eventPhotos.slice(photoIndex, photoIndex + 6) (or equivalent) and iterate over that slice in place of eventPhotos so you only render visible photos; keep the existing Wrapper selection (Wrapper = photo.href ? 'a' : 'div'), wrapperProps, Image props, key={photo.id}, and all classNames but remove the !isVisible hidden conditional since items outside the slice won't be rendered.
112-118: Usenext/linkfor internal navigation instead of<a>tags.Using plain
<a>tags for internal routes (e.g.,/events/rtsh) bypasses Next.js client-side navigation, causing full page reloads. Usenext/link'sLinkcomponent for better UX and performance.♻️ Proposed refactor
import { useState } from 'react' import Image from 'next/image' +import Link from 'next/link' ... - const Wrapper = photo.href ? 'a' : 'div' - const wrapperProps = photo.href ? { href: photo.href } : {} + const wrapperProps = { + className: `group relative overflow-hidden rounded-3xl aspect-video border border-white/10 hover:border-brand-pink/40 transition-all duration-300 block shadow-lg hover:shadow-brand-pink/10${!isVisible ? ' hidden' : ''}` + } + + if (photo.href) { + return ( + <Link key={photo.id} href={photo.href} {...wrapperProps}> + {/* Image and overlay content */} + </Link> + ) + } return ( - <Wrapper - key={photo.id} - {...wrapperProps} - className={`group relative overflow-hidden rounded-3xl aspect-video border border-white/10 hover:border-brand-pink/40 transition-all duration-300 block shadow-lg hover:shadow-brand-pink/10${!isVisible ? ' hidden' : ''}`} - > + <div key={photo.id} {...wrapperProps}> + {/* Image and overlay content */} + </div> )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/for-partners/PhotoGallery.tsx` around lines 112 - 118, The code uses a plain 'a' by setting Wrapper = photo.href ? 'a' : 'div', which loses Next.js client navigation for internal routes; change the logic to use next/link's Link for internal hrefs: detect isInternal = Boolean(photo.href && photo.href.startsWith('/')), set Wrapper = isInternal ? Link : (photo.href ? 'a' : 'div'), adjust wrapperProps to { href: photo.href } for Link/'a' and add target/rel for external links as needed, ensure you import Link from 'next/link' and keep passing className, key, and other props unchanged (update places referencing Wrapper and wrapperProps in the PhotoGallery component).app/for-partners/page.tsx (2)
226-230: Consider usingnext/imageinstead of native<img>for partner logos.The partner logo marquee uses native
<img>tags, missing Next.js image optimization benefits (lazy loading, format optimization, etc.). Since these are external URLs from NocoDB, you'll need to configureremotePatternsinnext.config.js.♻️ Proposed approach
If you add the NocoDB domain to
next.config.js:images: { remotePatterns: [ { protocol: 'https', hostname: 'ndb.startmunich.de' }, { protocol: 'https', hostname: 'ui-avatars.com' }, ], },Then replace:
- <img + <Image src={partner.logoUrl} alt={partner.name} - className="max-h-10 max-w-[100px] object-contain" + width={100} + height={40} + className="max-h-10 max-w-[100px] object-contain" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/for-partners/page.tsx` around lines 226 - 230, Replace the native <img> used to render partner logos with Next.js Image and configure remotePatterns for the external hosts; in next.config.js add the NocoDB and ui-avatars domains (e.g., ndb.startmunich.de and ui-avatars.com) under images.remotePatterns, then in app/for-partners/page.tsx import Image from 'next/image' and replace the <img src={partner.logoUrl} alt={partner.name} ... /> with <Image src={partner.logoUrl} alt={partner.name} className="..." /> using a supported sizing approach (provide width/height props or use fill with a positioned container) so partner.logoUrl and partner.name continue to be used and Next.js image optimizations apply.
210-215: Usenext/linkfor internal navigation.The
/partnerslinks use plain<a>tags, causing full page reloads. Use Next.jsLinkcomponent for client-side navigation.♻️ Proposed fix
Add import at the top:
import Link from 'next/link'Then update the links:
- <a + <Link href="/partners" className="hidden md:inline-flex px-6 py-2.5 border-2 border-brand-pink text-brand-pink hover:bg-brand-pink hover:text-white font-bold rounded-full transition-all duration-300 whitespace-nowrap text-center" > View All Partners → - </a> + </Link>- <a href="/partners" className="group inline-flex items-center gap-2 text-white/70 hover:text-brand-pink font-medium transition-colors"> + <Link href="/partners" className="group inline-flex items-center gap-2 text-white/70 hover:text-brand-pink font-medium transition-colors"> View all our partners <span className="group-hover:translate-x-1 transition-transform">→</span> - </a> + </Link>Also applies to: 238-241
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/for-partners/page.tsx` around lines 210 - 215, Import Link from 'next/link' and replace the plain anchor elements that navigate to "/partners" (the elements rendering the "View All Partners →" link at the shown locations) with Next.js Link components so navigation is client-side; keep the same href, className and inner text, and update both occurrences (the one around lines 210–215 and the one around 238–241) so they use <Link href="/partners"> (or Link wrapping an <a> if your Next version requires it) instead of a raw <a>.components/EventComponents.tsx (2)
245-246: Thumb calculations may produce incorrect values on initial render or window resize.
thumbWidthandthumbLeftare computed during render by accessingsliderRef.currentdirectly. On initial render, the ref may be null (producing 30/0 fallbacks). Also, these won't update on window resize since there's no re-render trigger.Consider using state initialized via
useEffector accept that the scroll event will re-render and correct the values.♻️ Optional: Add resize handling for more robust behavior
+import React, { useRef, useState, useEffect } from 'react' export const ScrollIndicator = ({ sliderRef, scrollProgress }: ScrollIndicatorProps) => { const trackRef = useRef<HTMLDivElement>(null) const isDragging = useRef(false) + const [dimensions, setDimensions] = useState({ clientWidth: 0, scrollWidth: 1 }) + + useEffect(() => { + const updateDimensions = () => { + if (sliderRef.current) { + setDimensions({ + clientWidth: sliderRef.current.clientWidth, + scrollWidth: sliderRef.current.scrollWidth + }) + } + } + updateDimensions() + window.addEventListener('resize', updateDimensions) + return () => window.removeEventListener('resize', updateDimensions) + }, [sliderRef]) - const thumbWidth = sliderRef.current ? (sliderRef.current.clientWidth / sliderRef.current.scrollWidth) * 100 : 30 - const thumbLeft = sliderRef.current ? scrollProgress * (1 - sliderRef.current.clientWidth / sliderRef.current.scrollWidth) : 0 + const thumbWidth = dimensions.scrollWidth > 0 ? (dimensions.clientWidth / dimensions.scrollWidth) * 100 : 30 + const thumbLeft = dimensions.scrollWidth > 0 ? scrollProgress * (1 - dimensions.clientWidth / dimensions.scrollWidth) : 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/EventComponents.tsx` around lines 245 - 246, thumbWidth and thumbLeft are being computed directly during render from sliderRef.current which can be null on first render and won't update on window resize; move these computations into a useEffect that runs when sliderRef.current, scrollProgress, or window size changes and store results in state (e.g., thumbWidthState and thumbLeftState) so the component re-renders with correct values; inside the effect guard sliderRef.current, compute width/left using sliderRef.current.clientWidth and scrollWidth, set fallback defaults only once if null, and optionally add a window "resize" listener to trigger the same effect cleanup to keep values correct on resize.
236-243: Potential memory leak if component unmounts during drag.If the user starts dragging and the component unmounts before
mouseup, the window event listeners won't be removed.♻️ Add cleanup on unmount
+import React, { useRef, useEffect } from 'react' export const ScrollIndicator = ({ sliderRef, scrollProgress }: ScrollIndicatorProps) => { const trackRef = useRef<HTMLDivElement>(null) const isDragging = useRef(false) + const cleanupRef = useRef<(() => void) | null>(null) + + useEffect(() => { + return () => { + if (cleanupRef.current) cleanupRef.current() + } + }, []) const handleMouseDown = (e: React.MouseEvent) => { isDragging.current = true scrollToRatio(getScrollRatio(e.clientX)) const onMove = (ev: MouseEvent) => { if (isDragging.current) scrollToRatio(getScrollRatio(ev.clientX)) } - const onUp = () => { isDragging.current = false; window.removeEventListener('mousemove', onMove); window.removeEventListener('mouseup', onUp) } + const onUp = () => { + isDragging.current = false + window.removeEventListener('mousemove', onMove) + window.removeEventListener('mouseup', onUp) + cleanupRef.current = null + } + cleanupRef.current = onUp window.addEventListener('mousemove', onMove) window.addEventListener('mouseup', onUp) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/EventComponents.tsx` around lines 236 - 243, handleMouseDown can register window listeners that won't be removed if the component unmounts during a drag; fix by storing the listeners in a ref and adding a useEffect cleanup that removes them on unmount. Concretely, create a ref like dragHandlersRef, assign dragHandlersRef.current = { onMove, onUp } inside handleMouseDown (use the existing onMove/onUp functions that call scrollToRatio/getScrollRatio and toggle isDragging), and add a useEffect which on cleanup checks dragHandlersRef.current and calls window.removeEventListener('mousemove', onMove) and window.removeEventListener('mouseup', onUp) and sets isDragging.current = false; this ensures the window listeners registered by handleMouseDown are always removed when the component unmounts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/for-partners/page.tsx`:
- Around line 445-458: The FAQ accordion icon isn't rotating because the span
with className "faq-icon" has transition-transform but lacks a group-open
Tailwind modifier; update the span (the element with className containing
"faq-icon") to include a group-open rotation class such as
"group-open:rotate-45" (or "group-open:rotate-180" if you prefer a full spin) so
it rotates when the parent <details> (which already has the "group" class) is
opened; alternatively, implement icon swapping by conditionally rendering "+" vs
"−" inside the same span if you prefer switching characters instead of rotating.
In `@app/for-partners/PhotoGallery.tsx`:
- Around line 40-41: Typo in PhotoGallery.tsx: update the href value(s) from
"/events/leagel-hack" to the correct "/events/legal-hack" wherever they appear
(e.g., the href property in the gallery item objects around the occurrences
shown); edit the href strings in the relevant JSX/objects so both instances (the
one at the first object and the second object) use "/events/legal-hack".
- Around line 85-86: The page counter calculation using Math.floor(photoIndex /
6) + 1 is out of sync with the navigation cap (photoIndex is clamped to
Math.max(0, eventPhotos.length - 6)), so compute the displayed page by checking
for the capped "last start" and otherwise using the normal division: let
totalPages = Math.ceil(eventPhotos.length / 6); let lastStart = Math.max(0,
eventPhotos.length - 6); let currentPage = photoIndex === lastStart ? totalPages
: Math.floor(photoIndex / 6) + 1; then use currentPage / totalPages in the span
(or alternatively maintain a dedicated pageIndex state updated in the next/prev
handlers). Reference symbols: photoIndex, eventPhotos.length, totalPages,
lastStart.
In `@components/CTA.tsx`:
- Around line 34-40: The CTA component currently always renders buttons with
Next.js Link, causing external URLs passed via button.href to behave like
internal routes; update the render logic in the CTA component to detect
absolute/external URLs (e.g., test button.href for scheme "http://" or
"https://" or new URL parsing) and for externals render a plain anchor element
with className constructed from baseClasses + (isPrimary ? primaryClasses :
secondaryClasses), plus target="_blank" and rel="noopener noreferrer"; continue
to render internal routes with the existing Link component to preserve client
routing. Ensure this logic references the existing symbols: CTA component,
button.href, baseClasses, primaryClasses, secondaryClasses, and Link.
---
Nitpick comments:
In `@app/for-partners/page.tsx`:
- Around line 226-230: Replace the native <img> used to render partner logos
with Next.js Image and configure remotePatterns for the external hosts; in
next.config.js add the NocoDB and ui-avatars domains (e.g., ndb.startmunich.de
and ui-avatars.com) under images.remotePatterns, then in
app/for-partners/page.tsx import Image from 'next/image' and replace the <img
src={partner.logoUrl} alt={partner.name} ... /> with <Image
src={partner.logoUrl} alt={partner.name} className="..." /> using a supported
sizing approach (provide width/height props or use fill with a positioned
container) so partner.logoUrl and partner.name continue to be used and Next.js
image optimizations apply.
- Around line 210-215: Import Link from 'next/link' and replace the plain anchor
elements that navigate to "/partners" (the elements rendering the "View All
Partners →" link at the shown locations) with Next.js Link components so
navigation is client-side; keep the same href, className and inner text, and
update both occurrences (the one around lines 210–215 and the one around
238–241) so they use <Link href="/partners"> (or Link wrapping an <a> if your
Next version requires it) instead of a raw <a>.
In `@app/for-partners/PhotoGallery.tsx`:
- Around line 109-141: The gallery currently maps over the entire eventPhotos
array and toggles visibility with the hidden class (using photoIndex), which
renders all items and harms performance; change the rendering to only map the
visible subset by computing a slice like eventPhotos.slice(photoIndex,
photoIndex + 6) (or equivalent) and iterate over that slice in place of
eventPhotos so you only render visible photos; keep the existing Wrapper
selection (Wrapper = photo.href ? 'a' : 'div'), wrapperProps, Image props,
key={photo.id}, and all classNames but remove the !isVisible hidden conditional
since items outside the slice won't be rendered.
- Around line 112-118: The code uses a plain 'a' by setting Wrapper = photo.href
? 'a' : 'div', which loses Next.js client navigation for internal routes; change
the logic to use next/link's Link for internal hrefs: detect isInternal =
Boolean(photo.href && photo.href.startsWith('/')), set Wrapper = isInternal ?
Link : (photo.href ? 'a' : 'div'), adjust wrapperProps to { href: photo.href }
for Link/'a' and add target/rel for external links as needed, ensure you import
Link from 'next/link' and keep passing className, key, and other props unchanged
(update places referencing Wrapper and wrapperProps in the PhotoGallery
component).
In `@components/EventComponents.tsx`:
- Around line 245-246: thumbWidth and thumbLeft are being computed directly
during render from sliderRef.current which can be null on first render and won't
update on window resize; move these computations into a useEffect that runs when
sliderRef.current, scrollProgress, or window size changes and store results in
state (e.g., thumbWidthState and thumbLeftState) so the component re-renders
with correct values; inside the effect guard sliderRef.current, compute
width/left using sliderRef.current.clientWidth and scrollWidth, set fallback
defaults only once if null, and optionally add a window "resize" listener to
trigger the same effect cleanup to keep values correct on resize.
- Around line 236-243: handleMouseDown can register window listeners that won't
be removed if the component unmounts during a drag; fix by storing the listeners
in a ref and adding a useEffect cleanup that removes them on unmount.
Concretely, create a ref like dragHandlersRef, assign dragHandlersRef.current =
{ onMove, onUp } inside handleMouseDown (use the existing onMove/onUp functions
that call scrollToRatio/getScrollRatio and toggle isDragging), and add a
useEffect which on cleanup checks dragHandlersRef.current and calls
window.removeEventListener('mousemove', onMove) and
window.removeEventListener('mouseup', onUp) and sets isDragging.current = false;
this ensures the window listeners registered by handleMouseDown are always
removed when the component unmounts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ea71da75-5233-493e-b107-7566f6c99040
📒 Files selected for processing (5)
app/for-partners/PhotoGallery.tsxapp/for-partners/page.tsxapp/member-journey/page.tsxcomponents/CTA.tsxcomponents/EventComponents.tsx
Comment on lines
+445
to
+458
| <details | ||
| key={faq.id} | ||
| className="bg-white/[0.06] rounded-2xl border border-white/10 overflow-hidden hover:border-brand-pink/30 transition-all duration-300" | ||
| className="group bg-white/[0.06] rounded-2xl border border-white/10 overflow-hidden hover:border-brand-pink/30 transition-all duration-300" | ||
| > | ||
| <button | ||
| onClick={() => setOpenFaq(openFaq === faq.id ? null : faq.id)} | ||
| className="w-full px-7 py-5 text-left flex items-center justify-between hover:bg-white/[0.04] transition-colors duration-200 rounded-2xl" | ||
| > | ||
| <summary className="w-full px-7 py-5 flex items-center justify-between cursor-pointer hover:bg-white/[0.04] transition-colors duration-200 rounded-2xl list-none"> | ||
| <h3 className="text-lg font-bold text-white pr-8">{faq.question}</h3> | ||
| <span className={`text-brand-pink text-2xl flex-shrink-0 transition-transform duration-300 ${openFaq === faq.id ? 'rotate-45' : ''}`}> | ||
| <span className="faq-icon text-brand-pink text-2xl flex-shrink-0 transition-transform duration-300"> | ||
| + | ||
| </span> | ||
| </button> | ||
| {openFaq === faq.id && ( | ||
| <div className="px-7 pb-5" style={{ animation: 'slideDown 0.3s ease-out' }}> | ||
| <p className="text-gray-400 leading-relaxed">{faq.answer}</p> | ||
| </div> | ||
| )} | ||
| </div> | ||
| </summary> | ||
| <div className="px-7 pb-5"> | ||
| <p className="text-gray-400 leading-relaxed">{faq.answer}</p> | ||
| </div> | ||
| </details> |
There was a problem hiding this comment.
FAQ accordion icon doesn't rotate when open.
The .faq-icon span has transition-transform but no Tailwind class to rotate when the <details> is open. Use group-open: modifier.
🐛 Proposed fix
- <span className="faq-icon text-brand-pink text-2xl flex-shrink-0 transition-transform duration-300">
- +
+ <span className="faq-icon text-brand-pink text-2xl flex-shrink-0 transition-transform duration-300 group-open:rotate-45">
+ +
</span>Alternatively, switch between + and − icons:
- <span className="faq-icon text-brand-pink text-2xl flex-shrink-0 transition-transform duration-300">
- +
- </span>
+ <span className="faq-icon text-brand-pink text-2xl flex-shrink-0">
+ <span className="group-open:hidden">+</span>
+ <span className="hidden group-open:inline">−</span>
+ </span>📝 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.
Suggested change
| <details | |
| key={faq.id} | |
| className="bg-white/[0.06] rounded-2xl border border-white/10 overflow-hidden hover:border-brand-pink/30 transition-all duration-300" | |
| className="group bg-white/[0.06] rounded-2xl border border-white/10 overflow-hidden hover:border-brand-pink/30 transition-all duration-300" | |
| > | |
| <button | |
| onClick={() => setOpenFaq(openFaq === faq.id ? null : faq.id)} | |
| className="w-full px-7 py-5 text-left flex items-center justify-between hover:bg-white/[0.04] transition-colors duration-200 rounded-2xl" | |
| > | |
| <summary className="w-full px-7 py-5 flex items-center justify-between cursor-pointer hover:bg-white/[0.04] transition-colors duration-200 rounded-2xl list-none"> | |
| <h3 className="text-lg font-bold text-white pr-8">{faq.question}</h3> | |
| <span className={`text-brand-pink text-2xl flex-shrink-0 transition-transform duration-300 ${openFaq === faq.id ? 'rotate-45' : ''}`}> | |
| <span className="faq-icon text-brand-pink text-2xl flex-shrink-0 transition-transform duration-300"> | |
| + | |
| </span> | |
| </button> | |
| {openFaq === faq.id && ( | |
| <div className="px-7 pb-5" style={{ animation: 'slideDown 0.3s ease-out' }}> | |
| <p className="text-gray-400 leading-relaxed">{faq.answer}</p> | |
| </div> | |
| )} | |
| </div> | |
| </summary> | |
| <div className="px-7 pb-5"> | |
| <p className="text-gray-400 leading-relaxed">{faq.answer}</p> | |
| </div> | |
| </details> | |
| <details | |
| key={faq.id} | |
| className="group bg-white/[0.06] rounded-2xl border border-white/10 overflow-hidden hover:border-brand-pink/30 transition-all duration-300" | |
| > | |
| <summary className="w-full px-7 py-5 flex items-center justify-between cursor-pointer hover:bg-white/[0.04] transition-colors duration-200 rounded-2xl list-none"> | |
| <h3 className="text-lg font-bold text-white pr-8">{faq.question}</h3> | |
| <span className="faq-icon text-brand-pink text-2xl flex-shrink-0 transition-transform duration-300 group-open:rotate-45"> | |
| </span> | |
| </summary> | |
| <div className="px-7 pb-5"> | |
| <p className="text-gray-400 leading-relaxed">{faq.answer}</p> | |
| </div> | |
| </details> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/for-partners/page.tsx` around lines 445 - 458, The FAQ accordion icon
isn't rotating because the span with className "faq-icon" has
transition-transform but lacks a group-open Tailwind modifier; update the span
(the element with className containing "faq-icon") to include a group-open
rotation class such as "group-open:rotate-45" (or "group-open:rotate-180" if you
prefer a full spin) so it rotates when the parent <details> (which already has
the "group" class) is opened; alternatively, implement icon swapping by
conditionally rendering "+" vs "−" inside the same span if you prefer switching
characters instead of rotating.
Comment on lines
+40
to
+41
| href: "/events/leagel-hack" | ||
| }, |
There was a problem hiding this comment.
Typo: "leagel-hack" should likely be "legal-hack".
The href /events/leagel-hack appears to have a typo — "leagel" instead of "legal".
🐛 Proposed fix
{
id: "ep6",
url: "/partners/partnerEvents/MHL1.jpg",
caption: "Munich Hacking Legal 25",
- href: "/events/leagel-hack"
+ href: "/events/legal-hack"
},
{
id: "ep7",
url: "/partners/partnerEvents/MHL2.jpg",
caption: "Munich Hacking Legal 25",
- href: "/events/leagel-hack"
+ href: "/events/legal-hack"
},Also applies to: 45-46
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/for-partners/PhotoGallery.tsx` around lines 40 - 41, Typo in
PhotoGallery.tsx: update the href value(s) from "/events/leagel-hack" to the
correct "/events/legal-hack" wherever they appear (e.g., the href property in
the gallery item objects around the occurrences shown); edit the href strings in
the relevant JSX/objects so both instances (the one at the first object and the
second object) use "/events/legal-hack".
Comment on lines
+85
to
+86
| <span className="text-sm text-gray-500 tabular-nums"> | ||
| {String(Math.floor(photoIndex / 6) + 1).padStart(2, '0')} / {String(Math.ceil(eventPhotos.length / 6)).padStart(2, '0')} |
There was a problem hiding this comment.
Page counter calculation is misaligned with navigation logic.
The page counter uses Math.floor(photoIndex / 6) + 1, but the "next" button caps photoIndex at eventPhotos.length - 6 (which is 4 for 10 photos). When photoIndex becomes 4, Math.floor(4/6) + 1 = 1, so the counter still shows "01" instead of "02".
Consider tracking page number directly or adjusting the counter calculation:
🐛 Proposed fix: Track page index directly
export default function PhotoGallery() {
- const [photoIndex, setPhotoIndex] = useState(0)
+ const [pageIndex, setPageIndex] = useState(0)
+ const pageSize = 6
+ const totalPages = Math.ceil(eventPhotos.length / pageSize)
+ const photoIndex = pageIndex * pageSize
return (
<section>
<div className="mb-12 flex flex-col md:flex-row md:items-end md:justify-between gap-6">
...
<div className="flex items-center gap-3 flex-shrink-0">
<span className="text-sm text-gray-500 tabular-nums">
- {String(Math.floor(photoIndex / 6) + 1).padStart(2, '0')} / {String(Math.ceil(eventPhotos.length / 6)).padStart(2, '0')}
+ {String(pageIndex + 1).padStart(2, '0')} / {String(totalPages).padStart(2, '0')}
</span>
<button
- onClick={() => setPhotoIndex(Math.max(0, photoIndex - 6))}
- disabled={photoIndex === 0}
+ onClick={() => setPageIndex(Math.max(0, pageIndex - 1))}
+ disabled={pageIndex === 0}
...
>
<button
- onClick={() => setPhotoIndex(Math.min(eventPhotos.length - 6, photoIndex + 6))}
- disabled={photoIndex + 6 >= eventPhotos.length}
+ onClick={() => setPageIndex(Math.min(totalPages - 1, pageIndex + 1))}
+ disabled={pageIndex >= totalPages - 1}
...
>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/for-partners/PhotoGallery.tsx` around lines 85 - 86, The page counter
calculation using Math.floor(photoIndex / 6) + 1 is out of sync with the
navigation cap (photoIndex is clamped to Math.max(0, eventPhotos.length - 6)),
so compute the displayed page by checking for the capped "last start" and
otherwise using the normal division: let totalPages =
Math.ceil(eventPhotos.length / 6); let lastStart = Math.max(0,
eventPhotos.length - 6); let currentPage = photoIndex === lastStart ? totalPages
: Math.floor(photoIndex / 6) + 1; then use currentPage / totalPages in the span
(or alternatively maintain a dedicated pageIndex state updated in the next/prev
handlers). Reference symbols: photoIndex, eventPhotos.length, totalPages,
lastStart.
Comment on lines
+34
to
+40
| <Link | ||
| key={index} | ||
| href={button.href} | ||
| target={button.external ? "_blank" : undefined} | ||
| rel={button.external ? "noopener noreferrer" : undefined} | ||
| className={`${baseClasses} ${isPrimary ? primaryClasses : secondaryClasses}`} | ||
| > | ||
| {button.label} | ||
| </a> | ||
| </Link> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all CTA component usages and check for external URLs
rg -n "href:\s*[\"']https?://" --type=ts --type=tsx -C3
rg -n "<CTA" --type=ts --type=tsx -A20 | rg -A5 "href.*http"Repository: startmunich/website-special-pages
Length of output: 132
🏁 Script executed:
#!/bin/bash
# First, find the CTA.tsx file and examine its content
find . -type f -name "CTA.tsx" | head -5Repository: startmunich/website-special-pages
Length of output: 97
🏁 Script executed:
#!/bin/bash
# Find all TypeScript/TSX files to understand the structure
git ls-files | grep -E '\.(tsx?|jsx?)$' | head -20Repository: startmunich/website-special-pages
Length of output: 611
🏁 Script executed:
#!/bin/bash
# Look for files containing "CTA" in their name or content
fd -e ts -e tsx | grep -i ctaRepository: startmunich/website-special-pages
Length of output: 95
🏁 Script executed:
#!/bin/bash
# Read the CTA.tsx file to understand current implementation
cat -n components/CTA.tsxRepository: startmunich/website-special-pages
Length of output: 3504
🏁 Script executed:
#!/bin/bash
# Find app/partners/page.tsx (the original review mentioned app/partners/page.tsx but I see app/for-partners/page.tsx)
find . -type f -name "*partners*page.tsx"Repository: startmunich/website-special-pages
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Check for both potential partner pages
cat -n app/for-partners/page.tsx | head -100Repository: startmunich/website-special-pages
Length of output: 4375
🏁 Script executed:
#!/bin/bash
# Search for CTA component usages throughout the codebase
rg "CTA\s*\{" --no-heading -A 10 | head -100Repository: startmunich/website-special-pages
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Search specifically for any href with http or https in CTA contexts
rg -A 20 "buttons\s*=" app/for-partners/page.tsx | head -80Repository: startmunich/website-special-pages
Length of output: 59
🏁 Script executed:
#!/bin/bash
# Search for CTA component import and usage
rg "import.*CTA" --type ts --type tsxRepository: startmunich/website-special-pages
Length of output: 104
🏁 Script executed:
#!/bin/bash
# Look for CTA usage more broadly
rg "<CTA" -B 2 -A 15 2>/dev/null | head -150Repository: startmunich/website-special-pages
Length of output: 3106
🏁 Script executed:
#!/bin/bash
# Check the entire for-partners page to find CTA usage
cat app/for-partners/page.tsx | tail -200Repository: startmunich/website-special-pages
Length of output: 10528
🏁 Script executed:
#!/bin/bash
# Verify if there are any other external URL usages with CTA
rg "href.*https?" app/startups/page.tsx app/member-journey/page.tsxRepository: startmunich/website-special-pages
Length of output: 236
🏁 Script executed:
#!/bin/bash
# Check the exact implementation in app/partners/page.tsx for the CTA usage
grep -n -A 10 "Contact Us" app/partners/page.tsxRepository: startmunich/website-special-pages
Length of output: 279
External URLs fail silently with <Link> component — must handle absolute URLs separately.
The CTA component unconditionally renders all buttons using Next.js <Link>, but app/partners/page.tsx (line 361) passes an external URL (https://tally.so/r/3xpGQG) to it. This causes:
- Navigation within the same tab instead of opening a new window
- Loss of
rel="noopener noreferrer"security attributes
Detect external URLs and use <a> tags for them instead.
Proposed fix
const renderButton = (button: CTAButton, index: number) => {
const isPrimary = button.variant === 'primary' || (index === 0 && !button.variant)
const baseClasses = "px-8 py-3 font-bold rounded-lg transition-all duration-300"
const primaryClasses = "bg-[`#d0006f`] hover:bg-[`#d0006f`]/90 text-white hover:shadow-lg hover:shadow-[`#d0006f`]/50"
const secondaryClasses = "border border-[`#d0006f`] text-[`#d0006f`] hover:bg-[`#d0006f`]/10"
+ const isExternal = button.href.startsWith('http://') || button.href.startsWith('https://')
+ if (isExternal) {
+ return (
+ <a
+ key={index}
+ href={button.href}
+ target="_blank"
+ rel="noopener noreferrer"
+ className={`${baseClasses} ${isPrimary ? primaryClasses : secondaryClasses}`}
+ >
+ {button.label}
+ </a>
+ )
+ }
return (
<Link
key={index}
href={button.href}
className={`${baseClasses} ${isPrimary ? primaryClasses : secondaryClasses}`}
>
{button.label}
</Link>
)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/CTA.tsx` around lines 34 - 40, The CTA component currently always
renders buttons with Next.js Link, causing external URLs passed via button.href
to behave like internal routes; update the render logic in the CTA component to
detect absolute/external URLs (e.g., test button.href for scheme "http://" or
"https://" or new URL parsing) and for externals render a plain anchor element
with className constructed from baseClasses + (isPrimary ? primaryClasses :
secondaryClasses), plus target="_blank" and rel="noopener noreferrer"; continue
to render internal routes with the existing Link component to preserve client
routing. Ensure this logic references the existing symbols: CTA component,
button.href, baseClasses, primaryClasses, secondaryClasses, and Link.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements