From e6e862485a88237389af0e0b6e6ae534fc17694c Mon Sep 17 00:00:00 2001 From: Aiden-Brine Date: Wed, 18 Feb 2026 17:09:44 -0800 Subject: [PATCH 1/3] Switch from state to ref --- packages/components/src/LightBox/LightBox.tsx | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/packages/components/src/LightBox/LightBox.tsx b/packages/components/src/LightBox/LightBox.tsx index 56fbeb45aa..2d3fb43fff 100644 --- a/packages/components/src/LightBox/LightBox.tsx +++ b/packages/components/src/LightBox/LightBox.tsx @@ -72,19 +72,15 @@ const swipePower = (offset: number, velocity: number) => { }; const variants = { - enter: (direction: number) => { - return { - x: direction > 0 ? "150%" : "-150%", - }; - }, + enter: (directionRef: React.RefObject) => ({ + x: directionRef.current > 0 ? "150%" : "-150%", + }), center: { x: 0, }, - exit: (direction: number) => { - return { - x: direction < 0 ? "150%" : "-150%", - }; - }, + exit: (directionRef: React.RefObject) => ({ + x: directionRef.current < 0 ? "150%" : "-150%", + }), }; const imageTransition = { @@ -105,7 +101,7 @@ export function LightBox({ onRequestClose, }: LightBoxProps) { const [currentImageIndex, setCurrentImageIndex] = useState(imageIndex); - const [direction, setDirection] = useState(0); + const directionRef = useRef(0); const [mouseIsStationary, setMouseIsStationary] = useState(true); const lightboxRef = useFocusTrap(open); const selectedThumbnailRef = useRef(null); @@ -194,7 +190,7 @@ export function LightBox({ key={currentImageIndex} variants={variants} src={images[currentImageIndex].url} - custom={direction} + custom={directionRef} className={styles.image} initial="enter" alt={ @@ -285,14 +281,14 @@ export function LightBox({ : template; function handleMovePrevious() { - setDirection(-1); + directionRef.current = -1; setCurrentImageIndex( (currentImageIndex + images.length - 1) % images.length, ); } function handleMoveNext() { - setDirection(1); + directionRef.current = 1; setCurrentImageIndex((currentImageIndex + 1) % images.length); } @@ -315,9 +311,9 @@ export function LightBox({ function handleThumbnailClick(index: number) { if (index < currentImageIndex) { - setDirection(-1); + directionRef.current = -1; } else { - setDirection(1); + directionRef.current = 1; } setCurrentImageIndex(index); } From ac75524d2176a714ef42926387037e8fabb4de45 Mon Sep 17 00:00:00 2001 From: Aiden-Brine Date: Wed, 18 Feb 2026 18:46:19 -0800 Subject: [PATCH 2/3] Add test and POM --- .../components/src/LightBox/LightBox.pom.tsx | 67 +++++++++++++++++++ .../components/src/LightBox/LightBox.test.tsx | 56 +++++++++++++--- 2 files changed, 112 insertions(+), 11 deletions(-) create mode 100644 packages/components/src/LightBox/LightBox.pom.tsx diff --git a/packages/components/src/LightBox/LightBox.pom.tsx b/packages/components/src/LightBox/LightBox.pom.tsx new file mode 100644 index 0000000000..3d46293f76 --- /dev/null +++ b/packages/components/src/LightBox/LightBox.pom.tsx @@ -0,0 +1,67 @@ +import { act, fireEvent, screen, within } from "@testing-library/react"; + +export function getCloseButton(): HTMLElement { + return screen.getByLabelText("Close"); +} + +export function getNextButton(): HTMLElement | null { + return screen.queryByLabelText("Next image"); +} + +export function getPreviousButton(): HTMLElement | null { + return screen.queryByLabelText("Previous image"); +} + +export function getThumbnailBar(): HTMLElement | null { + return screen.queryByTestId("ATL-Thumbnail-Bar"); +} + +export function getThumbnailByAlt(alt: string): HTMLElement { + const thumbnailBar = screen.getByTestId("ATL-Thumbnail-Bar"); + + return within(thumbnailBar).getByAltText(alt); +} + +export async function goNextThenPreviousWithRealTimers( + delayMs: number, +): Promise { + const delay = () => + act(async () => { + await new Promise(resolve => setTimeout(resolve, delayMs)); + }); + + fireEvent.click(screen.getByLabelText("Next image")); + await delay(); + fireEvent.click(screen.getByLabelText("Previous image")); + await delay(); +} + +export function getSlideImagesByAlt( + enteringAlt: string, + exitingAlt: string, +): { + enteringImg: HTMLElement | undefined; + exitingImg: HTMLElement | undefined; +} { + const lightbox = screen.getByLabelText("Lightbox"); + const thumbnailBar = screen.getByTestId("ATL-Thumbnail-Bar"); + const allImgs = within(lightbox).getAllByRole("img"); + const slideImgs = allImgs.filter(img => !thumbnailBar.contains(img)); + + return { + enteringImg: slideImgs.find(img => img.getAttribute("alt") === enteringAlt), + exitingImg: slideImgs.find(img => img.getAttribute("alt") === exitingAlt), + }; +} + +export function getSlideTransforms( + entering: HTMLElement, + exiting: HTMLElement, +): { enteringTransform: string; exitingTransform: string } { + return { + enteringTransform: + entering.style.transform || getComputedStyle(entering).transform, + exitingTransform: + exiting.style.transform || getComputedStyle(exiting).transform, + }; +} diff --git a/packages/components/src/LightBox/LightBox.test.tsx b/packages/components/src/LightBox/LightBox.test.tsx index feda829fad..4a655bea86 100644 --- a/packages/components/src/LightBox/LightBox.test.tsx +++ b/packages/components/src/LightBox/LightBox.test.tsx @@ -2,6 +2,7 @@ import React from "react"; import { fireEvent, render, screen } from "@testing-library/react"; import { BREAKPOINT_SIZES, mockViewportWidth } from "@jobber/hooks"; import { LightBox } from "."; +import * as POM from "./LightBox.pom"; const { setViewportWidth } = mockViewportWidth(); @@ -67,7 +68,7 @@ describe("LightBox", () => { expect(handleClose).toHaveBeenCalledTimes(1); }); - const { getByLabelText } = render( + render( { />, ); - fireEvent.click(getByLabelText("Close")); + fireEvent.click(POM.getCloseButton()); }); test("displays the image title of the selected imageIndex", () => { @@ -123,7 +124,7 @@ describe("LightBox", () => { { title: "title", caption: "caption", - url: "", + url: "https://example.com/photo.jpg", }, ], onRequestClose: jest.fn(), @@ -164,8 +165,8 @@ describe("LightBox", () => { onRequestClose={jest.fn()} />, ); - expect(screen.queryByLabelText("Previous image")).toBeInTheDocument(); - expect(screen.queryByLabelText("Next image")).toBeInTheDocument(); + expect(POM.getPreviousButton()).toBeInTheDocument(); + expect(POM.getNextButton()).toBeInTheDocument(); }); test("doesn't display the next and previous buttons when only one image", () => { @@ -183,8 +184,42 @@ describe("LightBox", () => { onRequestClose={jest.fn()} />, ); - expect(screen.queryByLabelText("Previous image")).toBeNull(); - expect(screen.queryByLabelText("Next image")).toBeNull(); + expect(POM.getPreviousButton()).toBeNull(); + expect(POM.getNextButton()).toBeNull(); + }); + + test("applies correct slide direction when changing from next to previous", async () => { + const images = [ + { title: "Title 0", alt: "alt-0", url: "https://example.com/0.jpg" }, + { title: "Title 1", alt: "alt-1", url: "https://example.com/1.jpg" }, + ]; + + render( + , + ); + + await POM.goNextThenPreviousWithRealTimers(250); + + await new Promise(resolve => setTimeout(resolve, 50)); + + const { enteringImg, exitingImg } = POM.getSlideImagesByAlt( + "alt-0", + "alt-1", + ); + expect(enteringImg).toBeDefined(); + expect(exitingImg).toBeDefined(); + + const { enteringTransform, exitingTransform } = POM.getSlideTransforms( + enteringImg as HTMLElement, + exitingImg as HTMLElement, + ); + expect(enteringTransform).toMatch(/translateX\(-/); + expect(exitingTransform).toMatch(/translateX\(\d/); }); }); @@ -216,7 +251,7 @@ describe("LightBox", () => { expect( screen.queryByAltText("alt of unselected image"), ).toBeInTheDocument(); - expect(screen.queryByTestId("ATL-Thumbnail-Bar")).toBeInTheDocument(); + expect(POM.getThumbnailBar()).toBeInTheDocument(); }); test("doesn't display when there is only one image", () => { @@ -236,7 +271,7 @@ describe("LightBox", () => { onRequestClose={handleClose} />, ); - expect(screen.queryByTestId("thumbnail-bar")).not.toBeInTheDocument(); + expect(POM.getThumbnailBar()).not.toBeInTheDocument(); }); test("displays the selected image thumbnail and caption when imageclicked", () => { @@ -269,8 +304,7 @@ describe("LightBox", () => { screen.queryByText(destinationImageCaption), ).not.toBeInTheDocument(); - const destinationImage = screen.getByAltText(destinationImageAlt); - fireEvent.click(destinationImage); + fireEvent.click(POM.getThumbnailByAlt(destinationImageAlt)); const imagesWithAlt = screen.getAllByAltText(destinationImageAlt); expect(imagesWithAlt).toHaveLength(2); From 8f0448777dc9587ac2d7b6bc5bd63e78f75e5a85 Mon Sep 17 00:00:00 2001 From: Aiden-Brine Date: Mon, 23 Feb 2026 10:45:19 -0800 Subject: [PATCH 3/3] Simplify test --- .../components/src/LightBox/LightBox.pom.tsx | 46 +------------------ .../components/src/LightBox/LightBox.test.tsx | 43 ++++++----------- packages/components/src/LightBox/LightBox.tsx | 4 +- 3 files changed, 17 insertions(+), 76 deletions(-) diff --git a/packages/components/src/LightBox/LightBox.pom.tsx b/packages/components/src/LightBox/LightBox.pom.tsx index 3d46293f76..58e58d92e3 100644 --- a/packages/components/src/LightBox/LightBox.pom.tsx +++ b/packages/components/src/LightBox/LightBox.pom.tsx @@ -1,4 +1,4 @@ -import { act, fireEvent, screen, within } from "@testing-library/react"; +import { screen, within } from "@testing-library/react"; export function getCloseButton(): HTMLElement { return screen.getByLabelText("Close"); @@ -21,47 +21,3 @@ export function getThumbnailByAlt(alt: string): HTMLElement { return within(thumbnailBar).getByAltText(alt); } - -export async function goNextThenPreviousWithRealTimers( - delayMs: number, -): Promise { - const delay = () => - act(async () => { - await new Promise(resolve => setTimeout(resolve, delayMs)); - }); - - fireEvent.click(screen.getByLabelText("Next image")); - await delay(); - fireEvent.click(screen.getByLabelText("Previous image")); - await delay(); -} - -export function getSlideImagesByAlt( - enteringAlt: string, - exitingAlt: string, -): { - enteringImg: HTMLElement | undefined; - exitingImg: HTMLElement | undefined; -} { - const lightbox = screen.getByLabelText("Lightbox"); - const thumbnailBar = screen.getByTestId("ATL-Thumbnail-Bar"); - const allImgs = within(lightbox).getAllByRole("img"); - const slideImgs = allImgs.filter(img => !thumbnailBar.contains(img)); - - return { - enteringImg: slideImgs.find(img => img.getAttribute("alt") === enteringAlt), - exitingImg: slideImgs.find(img => img.getAttribute("alt") === exitingAlt), - }; -} - -export function getSlideTransforms( - entering: HTMLElement, - exiting: HTMLElement, -): { enteringTransform: string; exitingTransform: string } { - return { - enteringTransform: - entering.style.transform || getComputedStyle(entering).transform, - exitingTransform: - exiting.style.transform || getComputedStyle(exiting).transform, - }; -} diff --git a/packages/components/src/LightBox/LightBox.test.tsx b/packages/components/src/LightBox/LightBox.test.tsx index 4a655bea86..82c01ded1d 100644 --- a/packages/components/src/LightBox/LightBox.test.tsx +++ b/packages/components/src/LightBox/LightBox.test.tsx @@ -2,6 +2,7 @@ import React from "react"; import { fireEvent, render, screen } from "@testing-library/react"; import { BREAKPOINT_SIZES, mockViewportWidth } from "@jobber/hooks"; import { LightBox } from "."; +import { slideVariants } from "./LightBox"; import * as POM from "./LightBox.pom"; const { setViewportWidth } = mockViewportWidth(); @@ -187,39 +188,23 @@ describe("LightBox", () => { expect(POM.getPreviousButton()).toBeNull(); expect(POM.getNextButton()).toBeNull(); }); + }); - test("applies correct slide direction when changing from next to previous", async () => { - const images = [ - { title: "Title 0", alt: "alt-0", url: "https://example.com/0.jpg" }, - { title: "Title 1", alt: "alt-1", url: "https://example.com/1.jpg" }, - ]; - - render( - , - ); - - await POM.goNextThenPreviousWithRealTimers(250); + describe("slideVariants", () => { + it("enter slides in from the right when direction is forward", () => { + expect(slideVariants.enter({ current: 1 })).toEqual({ x: "150%" }); + }); - await new Promise(resolve => setTimeout(resolve, 50)); + it("enter slides in from the left when direction is backward", () => { + expect(slideVariants.enter({ current: -1 })).toEqual({ x: "-150%" }); + }); - const { enteringImg, exitingImg } = POM.getSlideImagesByAlt( - "alt-0", - "alt-1", - ); - expect(enteringImg).toBeDefined(); - expect(exitingImg).toBeDefined(); + it("exit slides out to the left when direction is forward", () => { + expect(slideVariants.exit({ current: 1 })).toEqual({ x: "-150%" }); + }); - const { enteringTransform, exitingTransform } = POM.getSlideTransforms( - enteringImg as HTMLElement, - exitingImg as HTMLElement, - ); - expect(enteringTransform).toMatch(/translateX\(-/); - expect(exitingTransform).toMatch(/translateX\(\d/); + it("exit slides out to the right when direction is backward", () => { + expect(slideVariants.exit({ current: -1 })).toEqual({ x: "150%" }); }); }); diff --git a/packages/components/src/LightBox/LightBox.tsx b/packages/components/src/LightBox/LightBox.tsx index 2d3fb43fff..9f4573b485 100644 --- a/packages/components/src/LightBox/LightBox.tsx +++ b/packages/components/src/LightBox/LightBox.tsx @@ -71,7 +71,7 @@ const swipePower = (offset: number, velocity: number) => { return Math.abs(offset) * velocity; }; -const variants = { +export const slideVariants = { enter: (directionRef: React.RefObject) => ({ x: directionRef.current > 0 ? "150%" : "-150%", }), @@ -188,7 +188,7 @@ export function LightBox({