🚸 Add nav arrows in carousel modal #655
Conversation
williamchong
commented
Feb 1, 2026
williamchong
commented
There was a problem hiding this comment.
Pull request overview
This PR adds navigation arrows to the carousel modal, allowing users to navigate between carousel items directly from the modal view without closing it.
Changes:
- Replaced direct modal item ref with index-based tracking for navigation support
- Added left/right chevron navigation buttons to the modal with semi-transparent styling
- Implemented circular navigation logic that wraps around at carousel boundaries
| <UButton | ||
| icon="i-material-symbols-chevron-left" | ||
| color="neutral" | ||
| variant="ghost" | ||
| class="absolute left-2 top-1/2 -translate-y-1/2 rounded-full bg-black/50 text-white hover:bg-black/70 cursor-pointer" | ||
| @click="navigateModal(-1)" | ||
| /> | ||
| <UButton | ||
| icon="i-material-symbols-chevron-right" | ||
| color="neutral" | ||
| variant="ghost" | ||
| class="absolute right-2 top-1/2 -translate-y-1/2 rounded-full bg-black/50 text-white hover:bg-black/70 cursor-pointer" | ||
| @click="navigateModal(1)" | ||
| /> |
There was a problem hiding this comment.
The navigation arrows are always displayed, even when there's only one item in the carousel. The carousel itself only renders when carouselItems.length > 1 (line 3), but the modal with navigation arrows will still appear when clicking on a single BookCover (the else branch starting at line 115). The buttons should be conditionally rendered based on whether there are multiple items to navigate through.
| <UButton | ||
| icon="i-material-symbols-chevron-left" | ||
| color="neutral" | ||
| variant="ghost" | ||
| class="absolute left-2 top-1/2 -translate-y-1/2 rounded-full bg-black/50 text-white hover:bg-black/70 cursor-pointer" | ||
| @click="navigateModal(-1)" | ||
| /> | ||
| <UButton | ||
| icon="i-material-symbols-chevron-right" | ||
| color="neutral" | ||
| variant="ghost" | ||
| class="absolute right-2 top-1/2 -translate-y-1/2 rounded-full bg-black/50 text-white hover:bg-black/70 cursor-pointer" | ||
| @click="navigateModal(1)" | ||
| /> |
There was a problem hiding this comment.
The navigation buttons lack accessible labels for screen readers. Icon-only buttons should include aria-label attributes to describe their purpose for users relying on assistive technology. Add aria-label="Previous" to the left button and aria-label="Next" to the right button (or use appropriate internationalized strings).
components/BookCoverCarousel.vue
Outdated
| :alt="props.alt" | ||
| class="w-full h-auto max-h-[80vh] object-contain" | ||
| > | ||
| <div class="relative"> |
There was a problem hiding this comment.
why not add relative to content slot
| function navigateModal(direction: number) { | ||
| const len = carouselItems.value.length | ||
| modalItemIndex.value = (modalItemIndex.value + direction + len) % len | ||
| } |
There was a problem hiding this comment.
Consider adding keyboard navigation support (ArrowLeft/ArrowRight keys) for improved accessibility and user experience. The PDFReader component (components/PDFReader.vue:571-593) implements keyboard navigation using event listeners. This would allow users to navigate through carousel items using keyboard shortcuts in addition to clicking the arrow buttons.
| class="absolute left-2 top-1/2 -translate-y-1/2 rounded-full bg-black/50 text-white hover:bg-black/70 cursor-pointer" | ||
| @click="navigateModal(-1)" | ||
| /> | ||
| <UButton | ||
| icon="i-material-symbols-chevron-right" | ||
| color="neutral" | ||
| variant="ghost" | ||
| aria-label="Next" | ||
| class="absolute right-2 top-1/2 -translate-y-1/2 rounded-full bg-black/50 text-white hover:bg-black/70 cursor-pointer" | ||
| @click="navigateModal(1)" | ||
| /> |
There was a problem hiding this comment.
The cursor-pointer class is redundant for UButton components as buttons already have pointer cursor by default. This is a minor styling nit and doesn't affect functionality.
2 participants
