From 8f5e4cd9350b10a98afb7c98353c6719eee918bb Mon Sep 17 00:00:00 2001 From: Sean Perkins Date: Tue, 24 May 2022 16:22:53 -0400 Subject: [PATCH] fix(refresher): attach scroll listener to custom scroll target (#25335) Resolves #25318 --- core/src/components/refresher/refresher.tsx | 23 ++- .../components/refresher/test/basic/e2e.ts | 51 ------- .../refresher/test/basic/index.html | 2 +- .../refresher/test/basic/refresher.e2e.ts | 41 ++++++ .../refresher/test/scroll-target/e2e.ts | 47 ------- .../refresher/test/scroll-target/index.html | 6 +- .../test/scroll-target/refresher.e2e.ts | 42 ++++++ .../src/components/refresher/test/spec/e2e.ts | 10 -- .../components/refresher/test/spec/index.html | 131 ------------------ .../components/refresher/test/test.utils.ts | 27 +++- core/src/utils/content/index.ts | 4 +- .../utils/test/playwright/page/utils/goto.ts | 4 +- .../playwright/playwright-declarations.ts | 31 ++++- .../utils/test/playwright/playwright-page.ts | 44 +++--- 14 files changed, 188 insertions(+), 275 deletions(-) delete mode 100644 core/src/components/refresher/test/basic/e2e.ts create mode 100644 core/src/components/refresher/test/basic/refresher.e2e.ts delete mode 100644 core/src/components/refresher/test/scroll-target/e2e.ts create mode 100644 core/src/components/refresher/test/scroll-target/refresher.e2e.ts delete mode 100644 core/src/components/refresher/test/spec/e2e.ts delete mode 100644 core/src/components/refresher/test/spec/index.html diff --git a/core/src/components/refresher/refresher.tsx b/core/src/components/refresher/refresher.tsx index e62342dec9..56b85f7470 100644 --- a/core/src/components/refresher/refresher.tsx +++ b/core/src/components/refresher/refresher.tsx @@ -4,7 +4,12 @@ import { Component, Element, Event, Host, Method, Prop, State, Watch, h, readTas import { getIonMode } from '../../global/ionic-global'; import type { Animation, Gesture, GestureDetail, RefresherEventDetail } from '../../interface'; import { getTimeGivenProgression } from '../../utils/animation/cubic-bezier'; -import { findClosestIonContent, getScrollElement, printIonContentErrorMsg } from '../../utils/content'; +import { + getScrollElement, + ION_CONTENT_CLASS_SELECTOR, + ION_CONTENT_ELEMENT_SELECTOR, + printIonContentErrorMsg, +} from '../../utils/content'; import { clamp, getElementRoot, raf, transitionEndAsync } from '../../utils/helpers'; import { hapticImpact } from '../../utils/native/haptic'; @@ -436,13 +441,21 @@ export class Refresher implements ComponentInterface { return; } - const contentEl = findClosestIonContent(this.el); + const contentEl = this.el.closest(ION_CONTENT_ELEMENT_SELECTOR); if (!contentEl) { printIonContentErrorMsg(this.el); return; } - this.scrollEl = await getScrollElement(contentEl); + const customScrollTarget = contentEl.querySelector(ION_CONTENT_CLASS_SELECTOR); + + /** + * Query the custom scroll target (if available), first. In refresher implementations, + * the ion-refresher element will always be a direct child of ion-content (slot="fixed"). By + * querying the custom scroll target first and falling back to the ion-content element, + * the correct scroll element will be returned by the implementation. + */ + this.scrollEl = await getScrollElement(customScrollTarget ?? contentEl); /** * Query the host `ion-content` directly (if it is available), to use its @@ -452,9 +465,7 @@ export class Refresher implements ComponentInterface { * This makes it so that implementers do not need to re-create the background content * element and styles. */ - const backgroundContentHost = this.el.closest('ion-content') ?? contentEl; - - this.backgroundContentEl = getElementRoot(backgroundContentHost).querySelector( + this.backgroundContentEl = getElementRoot(contentEl ?? customScrollTarget).querySelector( '#background-content' ) as HTMLElement; diff --git a/core/src/components/refresher/test/basic/e2e.ts b/core/src/components/refresher/test/basic/e2e.ts deleted file mode 100644 index 86d9d9117f..0000000000 --- a/core/src/components/refresher/test/basic/e2e.ts +++ /dev/null @@ -1,51 +0,0 @@ -import type { E2EPage } from '@stencil/core/testing'; -import { newE2EPage } from '@stencil/core/testing'; - -import { pullToRefresh } from '../test.utils'; - -describe('refresher: basic', () => { - let page: E2EPage; - - beforeEach(async () => { - page = await newE2EPage({ - url: '/src/components/refresher/test/basic?ionic:_testing=true', - }); - }); - - it('should match existing visual screenshots', async () => { - const compare = await page.compareScreenshot(); - expect(compare).toMatchScreenshot(); - }); - - describe('legacy refresher', () => { - it('should load more items when performing a pull-to-refresh', async () => { - const initialItems = await page.findAll('ion-item'); - expect(initialItems.length).toBe(30); - - await pullToRefresh(page); - - const items = await page.findAll('ion-item'); - expect(items.length).toBe(60); - }); - }); - - describe('native refresher', () => { - it('should load more items when performing a pull-to-refresh', async () => { - const refresherContent = await page.$('ion-refresher-content'); - refresherContent.evaluate((el: any) => { - // Resets the pullingIcon to enable the native refresher - el.pullingIcon = undefined; - }); - - await page.waitForChanges(); - - const initialItems = await page.findAll('ion-item'); - expect(initialItems.length).toBe(30); - - await pullToRefresh(page); - - const items = await page.findAll('ion-item'); - expect(items.length).toBe(60); - }); - }); -}); diff --git a/core/src/components/refresher/test/basic/index.html b/core/src/components/refresher/test/basic/index.html index 51a6e3000f..60982df867 100644 --- a/core/src/components/refresher/test/basic/index.html +++ b/core/src/components/refresher/test/basic/index.html @@ -53,7 +53,7 @@ refresher.complete(); render(); // Custom event consumed by e2e tests - document.dispatchEvent(new CustomEvent('ionRefreshComplete')); + window.dispatchEvent(new CustomEvent('ionRefreshComplete')); }); function render() { diff --git a/core/src/components/refresher/test/basic/refresher.e2e.ts b/core/src/components/refresher/test/basic/refresher.e2e.ts new file mode 100644 index 0000000000..c67fca972a --- /dev/null +++ b/core/src/components/refresher/test/basic/refresher.e2e.ts @@ -0,0 +1,41 @@ +import { expect } from '@playwright/test'; +import { test } from '@utils/test/playwright'; + +import { pullToRefresh } from '../test.utils'; + +test.describe('refresher: basic', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/src/components/refresher/test/basic'); + }); + + test.describe('legacy refresher', () => { + test('should load more items when performing a pull-to-refresh', async ({ page }) => { + const items = page.locator('ion-item'); + + expect(await items.count()).toBe(30); + + await pullToRefresh(page); + + expect(await items.count()).toBe(60); + }); + }); + + test.describe('native refresher', () => { + test('should load more items when performing a pull-to-refresh', async ({ page }) => { + const refresherContent = page.locator('ion-refresher-content'); + refresherContent.evaluateHandle((el: any) => { + // Resets the pullingIcon to enable the native refresher + el.pullingIcon = undefined; + }); + + await page.waitForChanges(); + + const items = page.locator('ion-item'); + expect(await items.count()).toBe(30); + + await pullToRefresh(page); + + expect(await items.count()).toBe(60); + }); + }); +}); diff --git a/core/src/components/refresher/test/scroll-target/e2e.ts b/core/src/components/refresher/test/scroll-target/e2e.ts deleted file mode 100644 index 1920e75924..0000000000 --- a/core/src/components/refresher/test/scroll-target/e2e.ts +++ /dev/null @@ -1,47 +0,0 @@ -import type { E2EPage } from '@stencil/core/testing'; -import { newE2EPage } from '@stencil/core/testing'; - -import { pullToRefresh } from '../test.utils'; - -// TODO(FW-1134) Re-write these tests so that they test correct functionality. -describe.skip('refresher: custom scroll target', () => { - let page: E2EPage; - - beforeEach(async () => { - page = await newE2EPage({ - url: '/src/components/refresher/test/scroll-target?ionic:_testing=true', - }); - }); - - describe('legacy refresher', () => { - it('should load more items when performing a pull-to-refresh', async () => { - const initialItems = await page.findAll('ion-item'); - expect(initialItems.length).toBe(30); - - await pullToRefresh(page); - - const items = await page.findAll('ion-item'); - expect(items.length).toBe(60); - }); - }); - - describe('native refresher', () => { - it('should load more items when performing a pull-to-refresh', async () => { - const refresherContent = await page.$('ion-refresher-content'); - refresherContent.evaluate((el: any) => { - // Resets the pullingIcon to enable the native refresher - el.pullingIcon = undefined; - }); - - await page.waitForChanges(); - - const initialItems = await page.findAll('ion-item'); - expect(initialItems.length).toBe(30); - - await pullToRefresh(page); - - const items = await page.findAll('ion-item'); - expect(items.length).toBe(60); - }); - }); -}); diff --git a/core/src/components/refresher/test/scroll-target/index.html b/core/src/components/refresher/test/scroll-target/index.html index fe38d97e99..caa467fa3b 100644 --- a/core/src/components/refresher/test/scroll-target/index.html +++ b/core/src/components/refresher/test/scroll-target/index.html @@ -44,8 +44,8 @@ -
-
+
+
@@ -65,7 +65,7 @@ refresher.complete(); render(); // Custom event consumed by e2e tests - document.dispatchEvent(new CustomEvent('ionRefreshComplete')); + window.dispatchEvent(new CustomEvent('ionRefreshComplete')); }); function render() { diff --git a/core/src/components/refresher/test/scroll-target/refresher.e2e.ts b/core/src/components/refresher/test/scroll-target/refresher.e2e.ts new file mode 100644 index 0000000000..751de7c63f --- /dev/null +++ b/core/src/components/refresher/test/scroll-target/refresher.e2e.ts @@ -0,0 +1,42 @@ +import { expect } from '@playwright/test'; +import { test } from '@utils/test/playwright'; + +import { pullToRefresh } from '../test.utils'; + +test.describe('refresher: custom scroll target', () => { + test.beforeEach(async ({ page }) => { + await page.goto('/src/components/refresher/test/scroll-target'); + }); + + test.describe('legacy refresher', () => { + test('should load more items when performing a pull-to-refresh', async ({ page }) => { + const items = page.locator('ion-item'); + + expect(await items.count()).toBe(30); + + await pullToRefresh(page, '#inner-scroll'); + + expect(await items.count()).toBe(60); + }); + }); + + test.describe('native refresher', () => { + test('should load more items when performing a pull-to-refresh', async ({ page }) => { + const refresherContent = page.locator('ion-refresher-content'); + refresherContent.evaluateHandle((el: any) => { + // Resets the pullingIcon to enable the native refresher + el.pullingIcon = undefined; + }); + + await page.waitForChanges(); + + const items = page.locator('ion-item'); + + expect(await items.count()).toBe(30); + + await pullToRefresh(page, '#inner-scroll'); + + expect(await items.count()).toBe(60); + }); + }); +}); diff --git a/core/src/components/refresher/test/spec/e2e.ts b/core/src/components/refresher/test/spec/e2e.ts deleted file mode 100644 index 5171c5281a..0000000000 --- a/core/src/components/refresher/test/spec/e2e.ts +++ /dev/null @@ -1,10 +0,0 @@ -import { newE2EPage } from '@stencil/core/testing'; - -test('refresher: spec', async () => { - const page = await newE2EPage({ - url: '/src/components/refresher/test/spec?ionic:_testing=true', - }); - - const compare = await page.compareScreenshot(); - expect(compare).toMatchScreenshot(); -}); diff --git a/core/src/components/refresher/test/spec/index.html b/core/src/components/refresher/test/spec/index.html deleted file mode 100644 index 51e3e708b3..0000000000 --- a/core/src/components/refresher/test/spec/index.html +++ /dev/null @@ -1,131 +0,0 @@ - - - - - Refresher - Spec - - - - - - - - - - - - - - - - All Inboxes - - Edit - - - - - - - - - - - - All Inboxes - - - - - - - - - - - - - - - - - - - diff --git a/core/src/components/refresher/test/test.utils.ts b/core/src/components/refresher/test/test.utils.ts index 7d0732aeb5..40910ce864 100644 --- a/core/src/components/refresher/test/test.utils.ts +++ b/core/src/components/refresher/test/test.utils.ts @@ -1,5 +1,4 @@ -import type { E2EPage } from '@stencil/core/testing'; -import { dragElementBy } from '@utils/test'; +import type { E2EPage } from '@utils/test/playwright'; /** * Emulates a pull-to-refresh drag gesture (pulls down and releases). @@ -12,10 +11,28 @@ import { dragElementBy } from '@utils/test'; * @param selector The element selector to center the drag gesture on. Defaults to `body`. */ const pullToRefresh = async (page: E2EPage, selector = 'body') => { - const target = (await page.$(selector))!; + const target = page.locator(selector); - await dragElementBy(target, page, 0, 200); - const ev = await page.spyOnEvent('ionRefreshComplete', 'document'); + await page.waitForSelector('ion-refresher.hydrated', { state: 'attached' }); + + const ev = await page.spyOnEvent('ionRefreshComplete'); + const boundingBox = await target.boundingBox(); + + if (!boundingBox) { + return; + } + + const startX = boundingBox.x + boundingBox.width / 2; + const startY = boundingBox.y + boundingBox.height / 2; + + await page.mouse.move(startX, startY); + await page.mouse.down(); + + for (let i = 0; i < 400; i += 20) { + await page.mouse.move(startX, startY + i); + } + + await page.mouse.up(); await ev.next(); }; diff --git a/core/src/utils/content/index.ts b/core/src/utils/content/index.ts index 7e7a88bdf4..0d46e0fbfd 100644 --- a/core/src/utils/content/index.ts +++ b/core/src/utils/content/index.ts @@ -2,8 +2,8 @@ import { componentOnReady } from '../helpers'; import { printRequiredElementError } from '../logging'; const ION_CONTENT_TAG_NAME = 'ION-CONTENT'; -const ION_CONTENT_ELEMENT_SELECTOR = 'ion-content'; -const ION_CONTENT_CLASS_SELECTOR = '.ion-content-scroll-host'; +export const ION_CONTENT_ELEMENT_SELECTOR = 'ion-content'; +export const ION_CONTENT_CLASS_SELECTOR = '.ion-content-scroll-host'; /** * Selector used for implementations reliant on `` for scroll event changes. * diff --git a/core/src/utils/test/playwright/page/utils/goto.ts b/core/src/utils/test/playwright/page/utils/goto.ts index e0270935d9..f1667b259e 100644 --- a/core/src/utils/test/playwright/page/utils/goto.ts +++ b/core/src/utils/test/playwright/page/utils/goto.ts @@ -7,7 +7,7 @@ import type { Page, TestInfo } from '@playwright/test'; * automatically waits for the Stencil components * to be hydrated before proceeding with the test. */ -export const goto = async (page: Page, url: string, testInfo: TestInfo, originalFn: typeof page.goto) => { +export const goto = async (page: Page, url: string, options: any, testInfo: TestInfo, originalFn: typeof page.goto) => { const { mode, rtl, _testing } = testInfo.project.metadata; const splitUrl = url.split('?'); @@ -38,7 +38,7 @@ export const goto = async (page: Page, url: string, testInfo: TestInfo, original const result = await Promise.all([ page.waitForFunction(() => (window as any).testAppLoaded === true, { timeout: 4750 }), - originalFn(formattedUrl), + originalFn(formattedUrl, options), ]); return result[1]; diff --git a/core/src/utils/test/playwright/playwright-declarations.ts b/core/src/utils/test/playwright/playwright-declarations.ts index 0f56d701c4..e483724254 100644 --- a/core/src/utils/test/playwright/playwright-declarations.ts +++ b/core/src/utils/test/playwright/playwright-declarations.ts @@ -28,8 +28,35 @@ export interface E2EPage extends Page { * @param url URL to navigate page to. The url should include scheme, e.g. `https://`. When a `baseURL` via the context options was provided and the passed URL is a path, it gets merged via the * [`new URL()`](https://developer.mozilla.org/en-US/docs/Web/API/URL/URL) constructor. */ - goto: (url: string) => Promise; + goto: ( + url: string, + options?: { + /** + * Referer header value. If provided it will take preference over the referer header value set by + * [page.setExtraHTTPHeaders(headers)](https://playwright.dev/docs/api/class-page#page-set-extra-http-headers). + */ + referer?: string; + /** + * Maximum operation time in milliseconds, defaults to 30 seconds, pass `0` to disable timeout. The default value can be + * changed by using the + * [browserContext.setDefaultNavigationTimeout(timeout)](https://playwright.dev/docs/api/class-browsercontext#browser-context-set-default-navigation-timeout), + * [browserContext.setDefaultTimeout(timeout)](https://playwright.dev/docs/api/class-browsercontext#browser-context-set-default-timeout), + * [page.setDefaultNavigationTimeout(timeout)](https://playwright.dev/docs/api/class-page#page-set-default-navigation-timeout) + * or [page.setDefaultTimeout(timeout)](https://playwright.dev/docs/api/class-page#page-set-default-timeout) methods. + */ + timeout?: number; + + /** + * When to consider operation succeeded, defaults to `load`. Events can be either: + * - `'domcontentloaded'` - consider operation to be finished when the `DOMContentLoaded` event is fired. + * - `'load'` - consider operation to be finished when the `load` event is fired. + * - `'networkidle'` - consider operation to be finished when there are no network connections for at least `500` ms. + * - `'commit'` - consider operation to be finished when network response is received and the document started loading. + */ + waitUntil?: 'load' | 'domcontentloaded' | 'networkidle' | 'commit'; + } + ) => Promise; /** * Find an element by selector. * See https://playwright.dev/docs/locators for more information. @@ -58,9 +85,11 @@ export interface E2EPage extends Page { * never fires. * * Usage: + * ```ts * const ionChange = await page.spyOnEvent('ionChange'); * ... * await ionChange.next(); + * ``` */ spyOnEvent: (eventName: string) => Promise; _e2eEventsIds: number; diff --git a/core/src/utils/test/playwright/playwright-page.ts b/core/src/utils/test/playwright/playwright-page.ts index 5563aeca97..2af4fbfab1 100644 --- a/core/src/utils/test/playwright/playwright-page.ts +++ b/core/src/utils/test/playwright/playwright-page.ts @@ -31,24 +31,36 @@ type CustomFixtures = { page: E2EPage; }; +/** + * Extends the base `page` test figure within Playwright. + * @param page The page to extend. + * @param testInfo The test info. + * @returns The modified playwright page with extended functionality. + */ +export async function extendPageFixture(page: E2EPage, testInfo: TestInfo) { + const originalGoto = page.goto.bind(page); + const originalLocator = page.locator.bind(page); + + // Overridden Playwright methods + page.goto = (url: string, options) => goToPage(page, url, options, testInfo, originalGoto); + page.setContent = (html: string) => setContent(page, html); + page.locator = (selector: string, options?: LocatorOptions) => locator(page, originalLocator, selector, options); + + // Custom Ionic methods + page.getSnapshotSettings = () => getSnapshotSettings(page, testInfo); + page.setIonViewport = () => setIonViewport(page); + page.waitForChanges = (timeoutMs?: number) => waitForChanges(page, timeoutMs); + page.spyOnEvent = (eventName: string) => spyOnEvent(page, eventName); + + // Custom event behavior + await initPageEvents(page); + + return page; +} + export const test = base.extend({ page: async ({ page }: CustomTestArgs, use: (r: E2EPage) => Promise, testInfo: TestInfo) => { - const originalGoto = page.goto.bind(page); - const originalLocator = page.locator.bind(page); - - // Overridden Playwright methods - page.goto = (url: string) => goToPage(page, url, testInfo, originalGoto); - page.setContent = (html: string) => setContent(page, html); - page.locator = (selector: string, options?: LocatorOptions) => locator(page, originalLocator, selector, options); - // Custom Ionic methods - page.getSnapshotSettings = () => getSnapshotSettings(page, testInfo); - page.setIonViewport = () => setIonViewport(page); - page.waitForChanges = (timeoutMs?: number) => waitForChanges(page, timeoutMs); - page.spyOnEvent = (eventName: string) => spyOnEvent(page, eventName); - - // Custom event behavior - await initPageEvents(page); - + page = await extendPageFixture(page, testInfo); await use(page); }, });