Conversation
aariela
commented
Mar 12, 2026
aariela
commented
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a client-side “Partners” carousel component to the site, including a new arrow-button asset, to display partner logos with swipe support on mobile and arrow navigation on desktop.
Changes:
- Introduces
Partners.tsxcarousel component with desktop (3-up overlay) and mobile (single) layouts. - Adds touch swipe handling to navigate between partner logos.
- Adds a new
ArrowButton.pngasset for carousel navigation buttons.
Reviewed changes
Copilot reviewed 1 out of 5 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| apps/site/src/lib/components/Partners.tsx | New partners carousel component with swipe + arrow navigation and responsive rendering. |
| apps/site/src/assets/partner-icons/ArrowButton.png | New arrow button image used by the carousel navigation controls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button | ||
| onClick={handlePrev} | ||
| disabled={activeIndex === 0} | ||
| className="hidden md:block hover:opacity-70 transition-opacity disabled:opacity-30 disabled:cursor-not-allowed" | ||
| > | ||
| <Image | ||
| src={ArrowButton} |
There was a problem hiding this comment.
These <button>s don’t specify type. If this component is ever rendered inside a <form>, the default type="submit" can cause accidental submissions. Set type="button" for both arrow buttons.
| <div | ||
| className="absolute inset-[60px] pointer-events-none" | ||
| style={{ backgroundColor: "#88888863" }} | ||
| ></div> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
On mobile the navigation controls are entirely hidden (hidden md:block), leaving swipe as the only way to change slides. That makes the carousel unusable for keyboard-only and many assistive-technology users. Consider providing visible buttons on mobile as well (or at least an accessible control surface with proper labels).
| <button | ||
| onClick={handlePrev} | ||
| disabled={activeIndex === 0} | ||
| className="hidden md:block hover:opacity-70 transition-opacity disabled:opacity-30 disabled:cursor-not-allowed" | ||
| > | ||
| <Image | ||
| src={ArrowButton} |
There was a problem hiding this comment.
The arrow buttons rely on the <Image> alt text for labeling. For more reliable button accessibility, add an explicit aria-label on the <button> and make the arrow image decorative (e.g., empty alt), to avoid inconsistent screen reader output or duplicate announcements.
| className="absolute inset-[60px] pointer-events-none" | ||
| style={{ backgroundColor: "#88888863" }} | ||
| ></div> | ||
| </div> | ||
| </div> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
Same accessibility concern here: add an explicit aria-label on the <button> and make the arrow image decorative so assistive tech consistently announces the control as “Next/Previous” rather than depending on the image alt.
| const handleTouchStart = (e: TouchEvent) => { | ||
| touchStartX.current = e.touches[0].clientX; | ||
| }; | ||
|
|
||
| const handleTouchMove = (e: TouchEvent) => { | ||
| touchEndX.current = e.touches[0].clientX; | ||
| }; |
There was a problem hiding this comment.
touchEndX is never initialized/reset on touchstart, so a tap (no touchmove) or a new gesture after a prior swipe can compare touchStartX against a stale touchEndX value (default 0 or previous swipe) and trigger an unintended prev/next. Set touchEndX.current on touch start (and/or reset it on touch end/cancel) so handleTouchEnd only reacts to actual horizontal movement in the current gesture.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Add type='button' to all buttons - Add aria-labels for accessibility - Make arrow images decorative (empty alt) - Fix touch event handling to reset coordinates - Make overlay divs self-closing - Use consistent double quotes in styles
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
|
Deploy preview for venushacks ready!
|
