From 53ab71177e9573645c1a40c027b8f8c5b3e16efd Mon Sep 17 00:00:00 2001
From: shin-core <153108882+shin-core@users.noreply.github.com>
Date: Fri, 24 Jul 2026 19:36:12 +0900
Subject: [PATCH] fix(ui-kit): make Carousel's keyboard handler
orientation-aware
Carousel accepts orientation="horizontal"|"vertical" and every other part of the
component branches on it (content margins, item padding, prev/next button position
and rotation), but handleKeyDown was hardcoded to ArrowLeft/ArrowRight regardless.
On a vertical carousel -- whose prev/next buttons already point up/down -- ArrowUp/
ArrowDown did nothing while ArrowLeft/ArrowRight (meaningless in that layout) still
scrolled. Resolve orientation once (reusing the exact expression CarouselContext
already exposes) and bind ArrowUp/ArrowDown -> scrollPrev/scrollNext for vertical,
keeping ArrowLeft/ArrowRight for horizontal (the unchanged default). Adds carousel.tsx's
first tests: vertical uses Up/Down (not Left/Right), horizontal unchanged.
---
.../src/components/carousel.test.tsx | 75 +++++++++++++++++++
.../src/components/carousel.tsx | 21 ++++--
2 files changed, 91 insertions(+), 5 deletions(-)
create mode 100644 packages/loopover-ui-kit/src/components/carousel.test.tsx
diff --git a/packages/loopover-ui-kit/src/components/carousel.test.tsx b/packages/loopover-ui-kit/src/components/carousel.test.tsx
new file mode 100644
index 000000000..f45f4381a
--- /dev/null
+++ b/packages/loopover-ui-kit/src/components/carousel.test.tsx
@@ -0,0 +1,75 @@
+import { fireEvent, render, screen } from "@testing-library/react";
+import { afterEach, describe, expect, it, vi } from "vitest";
+
+// Mock Embla so we can assert the keyboard handler calls scrollPrev/scrollNext (#8308) without a real
+// carousel engine. useEmblaCarousel returns [refCallback, api]; the api only needs the members Carousel
+// touches (scroll + canScroll + the reInit/select event wiring in its effects).
+const scrollPrev = vi.fn();
+const scrollNext = vi.fn();
+const mockApi = {
+ scrollPrev,
+ scrollNext,
+ canScrollPrev: () => true,
+ canScrollNext: () => true,
+ on: vi.fn(),
+ off: vi.fn(),
+};
+vi.mock("embla-carousel-react", () => ({
+ default: () => [() => {}, mockApi],
+}));
+
+import { Carousel, CarouselContent, CarouselItem } from "./carousel";
+
+function renderCarousel(orientation: "horizontal" | "vertical") {
+ return render(
+
+
+ one
+ two
+
+ ,
+ );
+}
+
+afterEach(() => {
+ scrollPrev.mockClear();
+ scrollNext.mockClear();
+});
+
+describe("Carousel keyboard navigation is orientation-aware (#8308)", () => {
+ it("horizontal (default): ArrowLeft -> scrollPrev, ArrowRight -> scrollNext", () => {
+ renderCarousel("horizontal");
+ const region = screen.getByRole("region");
+ fireEvent.keyDown(region, { key: "ArrowLeft" });
+ expect(scrollPrev).toHaveBeenCalledTimes(1);
+ fireEvent.keyDown(region, { key: "ArrowRight" });
+ expect(scrollNext).toHaveBeenCalledTimes(1);
+ });
+
+ it("horizontal: ArrowUp/ArrowDown do NOT scroll", () => {
+ renderCarousel("horizontal");
+ const region = screen.getByRole("region");
+ fireEvent.keyDown(region, { key: "ArrowUp" });
+ fireEvent.keyDown(region, { key: "ArrowDown" });
+ expect(scrollPrev).not.toHaveBeenCalled();
+ expect(scrollNext).not.toHaveBeenCalled();
+ });
+
+ it("vertical: ArrowUp -> scrollPrev, ArrowDown -> scrollNext", () => {
+ renderCarousel("vertical");
+ const region = screen.getByRole("region");
+ fireEvent.keyDown(region, { key: "ArrowUp" });
+ expect(scrollPrev).toHaveBeenCalledTimes(1);
+ fireEvent.keyDown(region, { key: "ArrowDown" });
+ expect(scrollNext).toHaveBeenCalledTimes(1);
+ });
+
+ it("vertical: ArrowLeft/ArrowRight (no on-screen meaning in this layout) do NOT scroll", () => {
+ renderCarousel("vertical");
+ const region = screen.getByRole("region");
+ fireEvent.keyDown(region, { key: "ArrowLeft" });
+ fireEvent.keyDown(region, { key: "ArrowRight" });
+ expect(scrollPrev).not.toHaveBeenCalled();
+ expect(scrollNext).not.toHaveBeenCalled();
+ });
+});
diff --git a/packages/loopover-ui-kit/src/components/carousel.tsx b/packages/loopover-ui-kit/src/components/carousel.tsx
index 3e035ec14..154986c3d 100644
--- a/packages/loopover-ui-kit/src/components/carousel.tsx
+++ b/packages/loopover-ui-kit/src/components/carousel.tsx
@@ -83,17 +83,29 @@ const Carousel = React.forwardRef<
api?.scrollNext();
}, [api]);
+ // The single source of truth for orientation the CarouselContext already exposes -- reused here so the
+ // keyboard handler stays orientation-aware like every other part of the component (#8308), rather than a
+ // second independent check.
+ const resolvedOrientation =
+ orientation || (opts?.axis === "y" ? "vertical" : "horizontal");
+
const handleKeyDown = React.useCallback(
(event: React.KeyboardEvent) => {
- if (event.key === "ArrowLeft") {
+ // A vertical carousel's prev/next buttons already point up/down, so bind ArrowUp/ArrowDown to match;
+ // horizontal (the default) keeps ArrowLeft/ArrowRight. ArrowUp/ArrowLeft -> scrollPrev, the other -> scrollNext.
+ const prevKey =
+ resolvedOrientation === "vertical" ? "ArrowUp" : "ArrowLeft";
+ const nextKey =
+ resolvedOrientation === "vertical" ? "ArrowDown" : "ArrowRight";
+ if (event.key === prevKey) {
event.preventDefault();
scrollPrev();
- } else if (event.key === "ArrowRight") {
+ } else if (event.key === nextKey) {
event.preventDefault();
scrollNext();
}
},
- [scrollPrev, scrollNext],
+ [resolvedOrientation, scrollPrev, scrollNext],
);
React.useEffect(() => {
@@ -124,8 +136,7 @@ const Carousel = React.forwardRef<
carouselRef,
api: api,
opts,
- orientation:
- orientation || (opts?.axis === "y" ? "vertical" : "horizontal"),
+ orientation: resolvedOrientation,
scrollPrev,
scrollNext,
canScrollPrev,