From cbecafd587b606df128e0a377c364cdd9bc65959 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Thu, 18 Jul 2024 08:53:05 -0500 Subject: [PATCH 01/86] refactor: setup initial carousel testsw --- package.json | 2 +- .../carousel/carousel-container.tsx | 1 + .../src/components/carousel/carousel.css | 0 .../components/carousel/carousel.driver.ts | 47 +++++++++++++++++++ .../src/components/carousel/carousel.test.ts | 23 +++++++++ .../src/components/carousel/next-button.tsx | 1 + .../src/components/carousel/prev-button.tsx | 1 + .../src/components/carousel/slide.tsx | 1 + 8 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 packages/kit-headless/src/components/carousel/carousel.css create mode 100644 packages/kit-headless/src/components/carousel/carousel.driver.ts create mode 100644 packages/kit-headless/src/components/carousel/carousel.test.ts diff --git a/package.json b/package.json index 1ae34eeac..f9d20b78e 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "build.headless": "nx build headless", "change": "changeset", "changeset.version": "changeset version && pnpm install --no-frozen-lockfile && git add --all", - "dev": "nx serve website", + "dev": "nx serve website --host", "dev.ct": "nx serve component-tests", "format.all": "prettier --write \"**/*.{js,jsx,ts,tsx,json,html,css,scss}\"", "format.fix": "pretty-quick --staged", diff --git a/packages/kit-headless/src/components/carousel/carousel-container.tsx b/packages/kit-headless/src/components/carousel/carousel-container.tsx index c2ac651d3..fab50dfd8 100644 --- a/packages/kit-headless/src/components/carousel/carousel-container.tsx +++ b/packages/kit-headless/src/components/carousel/carousel-container.tsx @@ -14,6 +14,7 @@ export const HCarouselContainer = component$((props: CarouselContainerProps) => transitionDuration: `${context.transitionDurationSig.value}ms`, transitionDelay: '0ms', }} + data-qui-carousel-container {...props} > diff --git a/packages/kit-headless/src/components/carousel/carousel.css b/packages/kit-headless/src/components/carousel/carousel.css new file mode 100644 index 000000000..e69de29bb diff --git a/packages/kit-headless/src/components/carousel/carousel.driver.ts b/packages/kit-headless/src/components/carousel/carousel.driver.ts new file mode 100644 index 000000000..1b4eaf416 --- /dev/null +++ b/packages/kit-headless/src/components/carousel/carousel.driver.ts @@ -0,0 +1,47 @@ +import { Locator, Page } from '@playwright/test'; +export type DriverLocator = Locator | Page; + +export function createTestDriver(rootLocator: T) { + const getRoot = () => { + return rootLocator.locator('[data-qui-carousel]'); + }; + + const getNextButton = () => { + return getRoot().locator('[data-qui-carousel-next]'); + }; + + const getPrevButton = () => { + return getRoot().locator('[data-qui-carousel-prev]'); + }; + + const getContainer = () => { + return getRoot().locator('[data-qui-carousel-container]'); + }; + + const getSlideAt = (index: number) => { + return getContainer().locator(`[data-qui-carousel-slide]`).nth(index); + }; + + /** + * Wait for all animations within the given element and subtrees to finish + * See: https://github.com/microsoft/playwright/issues/15660#issuecomment-1184911658 + */ + function waitForAnimationEnd(selector: string) { + return getRoot() + .locator(selector) + .evaluate((element) => + Promise.all(element.getAnimations().map((animation) => animation.finished)), + ); + } + + return { + ...rootLocator, + locator: rootLocator, + getRoot, + getNextButton, + getPrevButton, + getContainer, + getSlideAt, + waitForAnimationEnd, + }; +} diff --git a/packages/kit-headless/src/components/carousel/carousel.test.ts b/packages/kit-headless/src/components/carousel/carousel.test.ts new file mode 100644 index 000000000..6100fe473 --- /dev/null +++ b/packages/kit-headless/src/components/carousel/carousel.test.ts @@ -0,0 +1,23 @@ +import { test, type Page } from '@playwright/test'; +import { createTestDriver } from './carousel.driver'; + +async function setup(page: Page, exampleName: string) { + await page.goto(`headless/carousel/${exampleName}`); + + const driver = createTestDriver(page); + + return { + driver, + }; +} + +test.describe('Mouse Behavior', () => { + test(`GIVEN a carousel + WHEN clicking on the next button + THEN it should move to the next slide + `, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + await d.getNextButton().click(); + }); +}); diff --git a/packages/kit-headless/src/components/carousel/next-button.tsx b/packages/kit-headless/src/components/carousel/next-button.tsx index 23e975dc1..1745bab96 100644 --- a/packages/kit-headless/src/components/carousel/next-button.tsx +++ b/packages/kit-headless/src/components/carousel/next-button.tsx @@ -16,6 +16,7 @@ export const HCarouselNext = component$((props: CarouselButtonProps) => { context.transitionDurationSig.value = 625; }} + data-qui-carousel-next > next slide diff --git a/packages/kit-headless/src/components/carousel/prev-button.tsx b/packages/kit-headless/src/components/carousel/prev-button.tsx index 30e6247fe..b082e5737 100644 --- a/packages/kit-headless/src/components/carousel/prev-button.tsx +++ b/packages/kit-headless/src/components/carousel/prev-button.tsx @@ -14,6 +14,7 @@ export const HCarouselPrev = component$((props: CarouselButtonProps) => { context.currentIndexSig.value--; context.transitionDurationSig.value = 625; }} + data-qui-carousel-prev > previous slide diff --git a/packages/kit-headless/src/components/carousel/slide.tsx b/packages/kit-headless/src/components/carousel/slide.tsx index dca53fb1b..91169b087 100644 --- a/packages/kit-headless/src/components/carousel/slide.tsx +++ b/packages/kit-headless/src/components/carousel/slide.tsx @@ -111,6 +111,7 @@ export const HCarouselSlide = component$(({ ...props }: CarouselSlideProps) => { data-slide-num={localIndexSig.value} style={{ marginRight: `${context.spaceBetweenSlides}px` }} ref={slideRef} + data-qui-carousel-slide {...props} > From 13f28629655ea26a09f9be60e99022b16bc0824d Mon Sep 17 00:00:00 2001 From: jack shelton Date: Thu, 18 Jul 2024 09:17:34 -0500 Subject: [PATCH 02/86] fix: carousel in CSR --- .../docs/headless/carousel/examples/csr.tsx | 58 +++++++++++++++++++ .../routes/docs/headless/carousel/index.mdx | 4 ++ .../components/carousel/carousel-viewport.tsx | 24 +------- 3 files changed, 63 insertions(+), 23 deletions(-) create mode 100644 apps/website/src/routes/docs/headless/carousel/examples/csr.tsx diff --git a/apps/website/src/routes/docs/headless/carousel/examples/csr.tsx b/apps/website/src/routes/docs/headless/carousel/examples/csr.tsx new file mode 100644 index 000000000..f0451f539 --- /dev/null +++ b/apps/website/src/routes/docs/headless/carousel/examples/csr.tsx @@ -0,0 +1,58 @@ +import { $, component$, useSignal, useStyles$ } from '@builder.io/qwik'; + +import { Carousel } from '@qwik-ui/headless'; +import carouselStyles from './carousel.css?inline'; + +export default component$(() => { + /* TODO: document this to always have initial state to null. + Use defaultSlide instead for setting a slide on page load */ + const currentIndexSig = useSignal(0); + const renderCarousel = useSignal(false); + useStyles$(carouselStyles); + + const nums = [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; + + return ( + <> + + {renderCarousel.value && ( + + + + + {nums.map((num, index) => ( + + {index} + + ))} + + +
+ { + return ( +
(currentIndexSig.value = i)} + > + {i} +
+ ); + })} + /> +
+
+ )} + + ); +}); diff --git a/apps/website/src/routes/docs/headless/carousel/index.mdx b/apps/website/src/routes/docs/headless/carousel/index.mdx index 8ecda9571..3fd815402 100644 --- a/apps/website/src/routes/docs/headless/carousel/index.mdx +++ b/apps/website/src/routes/docs/headless/carousel/index.mdx @@ -1,5 +1,7 @@ import { statusByComponent } from '~/_state/component-statuses'; + import { FeatureList } from '~/components/feature-list/feature-list'; + import { Note } from '~/components/note/note'; @@ -43,3 +45,5 @@ This component also intends to be more accessible with a powerful API. The component itself does include headless logic, but also uses the flex layout algorithm, and is in sort of a "gray" zone betweeen styled / unstyled. In the future, we would prefer a polished headless component (devoid of styles for most part). + + diff --git a/packages/kit-headless/src/components/carousel/carousel-viewport.tsx b/packages/kit-headless/src/components/carousel/carousel-viewport.tsx index 3925c7e67..4d479cd74 100644 --- a/packages/kit-headless/src/components/carousel/carousel-viewport.tsx +++ b/packages/kit-headless/src/components/carousel/carousel-viewport.tsx @@ -1,33 +1,11 @@ -import { - component$, - type PropsOf, - Slot, - useContext, - $, - useSignal, - useTask$, -} from '@builder.io/qwik'; +import { component$, type PropsOf, Slot, useContext, $ } from '@builder.io/qwik'; import CarouselContextId from './carousel-context-id'; -import { isBrowser } from '@builder.io/qwik/build'; type CarouselViewportProps = PropsOf<'div'>; export const HCarouselView = component$((props: CarouselViewportProps) => { const context = useContext(CarouselContextId); - const totalWidthSig = useSignal(0); - - useTask$(({ track }) => { - track(() => context.isDraggingSig.value); - - if (isBrowser) { - totalWidthSig.value = - context.numSlidesSig.value * - context.slideRefsArray.value[0].value.offsetWidth * - -1; - } - }); - const handlePointerMove$ = $((e: PointerEvent) => { if (context.isDraggingSig.value) { if (!context.containerRef.value) { From 7650e9755743039fae31b5a1d5bb76656d4a1f5c Mon Sep 17 00:00:00 2001 From: jack shelton Date: Thu, 18 Jul 2024 12:32:03 -0500 Subject: [PATCH 03/86] add expectation --- .../kit-headless/src/components/carousel/carousel.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/kit-headless/src/components/carousel/carousel.test.ts b/packages/kit-headless/src/components/carousel/carousel.test.ts index 6100fe473..0649f58b4 100644 --- a/packages/kit-headless/src/components/carousel/carousel.test.ts +++ b/packages/kit-headless/src/components/carousel/carousel.test.ts @@ -1,4 +1,4 @@ -import { test, type Page } from '@playwright/test'; +import { test, type Page, expect } from '@playwright/test'; import { createTestDriver } from './carousel.driver'; async function setup(page: Page, exampleName: string) { @@ -19,5 +19,7 @@ test.describe('Mouse Behavior', () => { const { driver: d } = await setup(page, 'hero'); await d.getNextButton().click(); + + await expect(d.getSlideAt(1)).toBeVisible(); }); }); From 8248ff84e73e43465cdb246163cce4b27d352d6f Mon Sep 17 00:00:00 2001 From: jack shelton Date: Thu, 18 Jul 2024 16:21:43 -0500 Subject: [PATCH 04/86] test: added test cases --- .../src/components/carousel/carousel.test.ts | 254 ++++++++++++++++++ .../src/components/carousel/carousel.tsx | 2 +- 2 files changed, 255 insertions(+), 1 deletion(-) diff --git a/packages/kit-headless/src/components/carousel/carousel.test.ts b/packages/kit-headless/src/components/carousel/carousel.test.ts index 0649f58b4..835137665 100644 --- a/packages/kit-headless/src/components/carousel/carousel.test.ts +++ b/packages/kit-headless/src/components/carousel/carousel.test.ts @@ -1,5 +1,6 @@ import { test, type Page, expect } from '@playwright/test'; import { createTestDriver } from './carousel.driver'; +import { AxeBuilder } from '@axe-core/playwright'; async function setup(page: Page, exampleName: string) { await page.goto(`headless/carousel/${exampleName}`); @@ -16,10 +17,263 @@ test.describe('Mouse Behavior', () => { WHEN clicking on the next button THEN it should move to the next slide `, async ({ page }) => { + /* + example that gets used goes here. In this case it's hero from: + apps/website/src/routes/docs/headless/carousel/examples/hero.tsx + + If you type in 'test' in the setup parameter it will look for the test.tsx file + */ const { driver: d } = await setup(page, 'hero'); await d.getNextButton().click(); await expect(d.getSlideAt(1)).toBeVisible(); }); + + test(`GIVEN a carousel + WHEN clicking on the previous button + THEN it should move to the previous slide + `, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // initial setup (if this gets used often we can make it a function in dthe drriver) + await d.getNextButton().click(); + await expect(d.getSlideAt(1)).toBeVisible(); + + // test previous work + }); + + test(`GIVEN a carousel with dragging enabled + WHEN using a pointer device and dragging to the left + THEN it should move to the next slide + `, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with dragging enabled + WHEN using a pointer device and dragging to the right + THEN it should move to the previous slide +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with a pagination control + WHEN clicking on the pagination bullets + THEN it should move to the corresponding slide +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); +}); + +test.describe('Keyboard Behavior', () => { + test(`GIVEN a carousel + WHEN the enter key is pressed on the focused next button + THEN it should move to the next slide + `, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + await d.getNextButton().press('Enter'); + + await expect(d.getSlideAt(1)).toBeVisible(); + }); + + test(`GIVEN a carousel + WHEN the enter key is pressed on the focused previous button + THEN it should move to the previous slide + `, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with a pagination control + WHEN the first bullet is focused and the right arrow key is pressed + THEN focus should move to the next bullet +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with a pagination control + WHEN the 2nd bullet is focused and the left arrow key is pressed + THEN focus should move to the previous bullet +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); +}); + +test.describe('Mobile / Touch Behavior', () => { + test(`GIVEN a carousel with dragging enabled + WHEN swiping to the left + THEN it should move to the next slide +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with dragging enabled + WHEN swiping to the right + THEN it should move to the previous slide +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with a pagination control + WHEN tapping on the pagination bullets + THEN it should move to the corresponding slide +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); }); + +test.describe('Accessibility', () => { + test('Axe Validation Test', async ({ page }) => { + await setup(page, 'hero'); + + const initialResults = await new AxeBuilder({ page }) + .include('[data-qui-carousel]') + .analyze(); + + expect(initialResults.violations).toEqual([]); + }); + + test(`GIVEN a carousel + WHEN it is rendered + THEN it should have an accessible name +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel + WHEN it is rendered + THEN the carousel container should have the role of group +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel + WHEN it is rendered + THEN the items should have a posinset of its current index +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with a pagination control + WHEN it is rendered + THEN the control should have the role of navigation +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with a pagination control + WHEN it is rendered + THEN each bullet should have the role of tab +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + // it should also have aria-live polite and announce the current slide + + test(`GIVEN a carousel + WHEN a slide is not the current slide + THEN it should be inert +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel + WHEN on the current slide + THEN items inside the slide should be the only focusable items +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); +}); + +// TODO: finish test cases, create new ones based on the expected behavior in Figma. + +// Getting a failing test when the test is expected to work helps us find bugs. + +// https://www.w3.org/WAI/ARIA/apg/patterns/carousel/ <-- another good resource for what functionality is expected + +/** + * + * When there is a use case that the default hero.tsx example doesn't cover, add a new test file in the docs headless/carousel/examples folder. + * + * + */ diff --git a/packages/kit-headless/src/components/carousel/carousel.tsx b/packages/kit-headless/src/components/carousel/carousel.tsx index c4ece9c04..2ce0cd43a 100644 --- a/packages/kit-headless/src/components/carousel/carousel.tsx +++ b/packages/kit-headless/src/components/carousel/carousel.tsx @@ -51,7 +51,7 @@ export const HCarousel = component$( useContextProvider(CarouselContextId, context); return ( -
+
Slide {context.currentIndexSig.value} of {context.numSlidesSig.value} From f1d02fb7dfae723dc7899682dde155588f7490c7 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Thu, 18 Jul 2024 21:43:40 -0500 Subject: [PATCH 05/86] test: some additional test cases --- .../src/components/carousel/carousel.test.ts | 74 +++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/packages/kit-headless/src/components/carousel/carousel.test.ts b/packages/kit-headless/src/components/carousel/carousel.test.ts index 835137665..34acd154e 100644 --- a/packages/kit-headless/src/components/carousel/carousel.test.ts +++ b/packages/kit-headless/src/components/carousel/carousel.test.ts @@ -263,6 +263,80 @@ test.describe('Accessibility', () => { // TODO }); + + test(`GIVEN a carousel with loop disabled + WHEN on the last slide + THEN the previous button should be focused +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with loop disabled + WHEN on the first slide + THEN the next button should be focused +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); +}); + +test.describe('Behavior', () => { + test(`GIVEN a carousel with loop disabled + WHEN on the last slide + THEN the next button should be disabled +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with loop disabled + WHEN on the first slide + THEN the previous button should be disabled +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with loop enabled + WHEN on the last slide and the next button is clicked + THEN it should move to the first slide +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); + + test(`GIVEN a carousel with loop enabled + WHEN on the first slide and the previous button is clicked + THEN it should move to the first slide +`, async ({ page }) => { + const { driver: d } = await setup(page, 'hero'); + + // remove this (there so that TS doesn't complain) + d; + + // TODO + }); }); // TODO: finish test cases, create new ones based on the expected behavior in Figma. From 5a652167772086a9f9b022ea4194853f0b10e225 Mon Sep 17 00:00:00 2001 From: jack shelton Date: Thu, 18 Jul 2024 21:57:23 -0500 Subject: [PATCH 06/86] not draggable --- .../src/routes/docs/headless/carousel/examples/hero.tsx | 1 + .../src/components/carousel/carousel-context.type.ts | 4 +++- .../src/components/carousel/carousel-viewport.tsx | 6 ++++-- .../kit-headless/src/components/carousel/carousel.css | 8 ++++++++ .../kit-headless/src/components/carousel/carousel.tsx | 7 +++++-- packages/kit-headless/src/components/carousel/slide.tsx | 6 +++--- 6 files changed, 24 insertions(+), 8 deletions(-) diff --git a/apps/website/src/routes/docs/headless/carousel/examples/hero.tsx b/apps/website/src/routes/docs/headless/carousel/examples/hero.tsx index 0eaa8a7bf..96eeaaeba 100644 --- a/apps/website/src/routes/docs/headless/carousel/examples/hero.tsx +++ b/apps/website/src/routes/docs/headless/carousel/examples/hero.tsx @@ -97,6 +97,7 @@ export default component$(() => { bind:currSlideIndex={currentIndexSig} spaceBetweenSlides={30} class="carousel" + draggable={false} >