DATASHARE-134: FE: Mobile/Tablet Homepage: Latest Updates#90
DATASHARE-134: FE: Mobile/Tablet Homepage: Latest Updates#90RuoxiZhang08 wants to merge 14 commits into1.2.0from
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a mobile/tablet-friendly “Latest Updates” experience on the Landing Page by introducing a mobile carousel with auto-rotation and updating card layouts/styles to match the new responsive design requirements.
Changes:
- Implemented a mobile-only rotating carousel (auto-advance + prev/next + pause/play) for the Gallery section.
- Updated
UpdateCardlayout/styling across breakpoints, including a hover overlay behavior for tablet/mobile and a desktop-specific layout. - Introduced a new Tailwind v4 custom breakpoint token (
--breakpoint-gallery-lg) and added new start/pause SVG icons.
Reviewed changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/components/LandingPage/Gallery/index.tsx |
Adds mobile carousel logic/state, new layout wrappers, and updated spacing/alignment rules. |
src/components/LandingPage/Gallery/UpdateCard.tsx |
Redesigns update cards per breakpoint, adds hover overlay behavior, and updates typography/colors. |
src/app/globals.css |
Defines a new Tailwind v4 breakpoint token used by gallery-lg:* responsive classes. |
assets/icons/Start_Icon.svg |
Adds play/start icon for the carousel control. |
assets/icons/Pause_Icon.svg |
Adds pause icon for the carousel control. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| const [slideTransitionEnabled, setSlideTransitionEnabled] = useState(true); | ||
| const [isMobileViewport, setIsMobileViewport] = useState(false); | ||
| const slideDirectionRef = useRef<"next" | "prev" | null>(null); | ||
| const slideTrackRef = useRef<HTMLDivElement>(null); |
There was a problem hiding this comment.
slideTrackRef is declared but never read. With Next/TypeScript ESLint rules this is typically flagged as an unused variable and can fail CI; remove it (and the ref={...}) or use it (e.g., for imperative scrolling/measurement).
| const slideTrackRef = useRef<HTMLDivElement>(null); |
| aria-label={isPaused ? "Play carousel" : "Pause carousel"} | ||
| className="flex items-center justify-center rounded-full border border-[#4BBFC6] bg-transparent w-[39.158px] h-[39.158px] text-[#6B7280] touch-manipulation cursor-pointer shadow-[0px_2.49px_9.32px_0px_#00000073]" | ||
| > | ||
| <img src={isPaused ? startIcon.src : pauseIcon.src} alt="Pause" className={isPaused ? "w-[20px] h-[22px] pl-[4px]" : "w-[18px] h-[18px]"} /> |
There was a problem hiding this comment.
The play/pause icon image always uses alt="Pause", even when the carousel is paused and the button’s aria-label switches to “Play carousel”. This makes the control misleading for screen readers; make the alt text reflect the current state (or use empty alt if the button already has an accurate aria-label).
| <img src={isPaused ? startIcon.src : pauseIcon.src} alt="Pause" className={isPaused ? "w-[20px] h-[22px] pl-[4px]" : "w-[18px] h-[18px]"} /> | |
| <img src={isPaused ? startIcon.src : pauseIcon.src} alt="" className={isPaused ? "w-[20px] h-[22px] pl-[4px]" : "w-[18px] h-[18px]"} /> |
| /** Build initial mobile list [c, a, b, c, a, b] from updates [a, b, c] so visible indices 1,2,3 show a,b,c */ | ||
| function buildMobileRotatingList(updates: GalleryUpdate[] | undefined): GalleryUpdate[] { | ||
| if (!updates || updates.length < 3) return []; | ||
| const [a, b, c] = updates; | ||
| return [c, a, b, c, a, b]; |
There was a problem hiding this comment.
buildMobileRotatingList only works for exactly 3 updates (it takes [a,b,c] and returns a hard-coded 6-item list). If config.updates ever contains <3 or >3 items (data-driven from home.json), the mobile carousel will render empty/incorrectly while still showing controls. Consider supporting arbitrary lengths (e.g., build a circular buffer from the full list) and/or conditionally hiding the mobile carousel + controls when there aren’t enough items to slide.
| /** Build initial mobile list [c, a, b, c, a, b] from updates [a, b, c] so visible indices 1,2,3 show a,b,c */ | |
| function buildMobileRotatingList(updates: GalleryUpdate[] | undefined): GalleryUpdate[] { | |
| if (!updates || updates.length < 3) return []; | |
| const [a, b, c] = updates; | |
| return [c, a, b, c, a, b]; | |
| /** Build initial mobile list for the mobile carousel. | |
| * - For exactly 3 updates [a, b, c], return [c, a, b, c, a, b] so visible indices 1,2,3 show a,b,c. | |
| * - For fewer than 3 updates, return [] (not enough items to slide). | |
| * - For more than 3 updates, return the full list unchanged. | |
| */ | |
| function buildMobileRotatingList(updates: GalleryUpdate[] | undefined): GalleryUpdate[] { | |
| if (!updates || updates.length < 3) { | |
| return []; | |
| } | |
| if (updates.length === 3) { | |
| const [a, b, c] = updates; | |
| return [c, a, b, c, a, b]; | |
| } | |
| // For more than 3 updates, use the entire list without truncation. | |
| return updates; |
| {/* Tablet/mobile only: hover overlay per Figma 2174-9260 — dark overlay, centered text, teal Read More */} | ||
| <div | ||
| className="absolute inset-0 rounded-[inherit] bg-black/70 flex flex-col justify-center p-4 opacity-0 transition-opacity duration-200 group-hover:opacity-100 z-[1] gallery-lg:hidden" | ||
| aria-hidden | ||
| > | ||
| <p className="font-inter font-normal text-white text-[14px] leading-[20px] line-clamp-5 mb-3 text-left"> | ||
| {description.length > 100 | ||
| ? `${description.slice(0, 100)}...` | ||
| : description} | ||
| </p> | ||
| <a | ||
| href={link} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| className="font-inter font-normal text-[14px] leading-[22px] text-[#4BBFC6] underline decoration-[#4BBFC6] hover:text-[#72f9fb] hover:decoration-[#72f9fb] cursor-pointer w-fit text-left" | ||
| > | ||
| Read More > | ||
| </a> |
There was a problem hiding this comment.
The hover overlay is hidden via opacity-0 but still contains a focusable <a>; keyboard users can tab to an invisible link (and aria-hidden does not prevent focus). This is an accessibility issue. Consider disabling pointer events while hidden (e.g., pointer-events-none + enabling on group-hover/group-focus-within), and ensure the overlay can also be revealed on keyboard focus (not hover-only).
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
No description provided.