Skip to content
Merged
34 changes: 25 additions & 9 deletions core/src/components/item-sliding/test/basic/item-sliding.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
import { expect } from '@playwright/test';
import { configs, dragElementBy, test } from '@utils/test/playwright';

import {
DRAG_DISTANCE_SINGLE_OPTION,
DRAG_DISTANCE_MULTIPLE_OPTIONS,
DRAG_STEPS_UNDER_FULL_SWIPE,
} from '../test.utils';

/**
* item-sliding doesn't have mode-specific styling,
* but the child components, item-options and item-option, do.
Expand All @@ -23,9 +29,9 @@ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, conf
* Positive dragByX value to drag element from the left to the right
* to reveal the options on the left side.
*/
const dragByX = config.direction === 'rtl' ? -100 : 100;
const dragByX = config.direction === 'rtl' ? -DRAG_DISTANCE_SINGLE_OPTION : DRAG_DISTANCE_SINGLE_OPTION;

await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
await page.waitForChanges();

await expect(item).toHaveScreenshot(screenshot('item-sliding-start'));
Expand All @@ -42,9 +48,10 @@ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, conf
* Positive dragByX value to drag element from the left to the right
* to reveal the options on the left side.
*/
const dragByX = config.direction === 'rtl' ? 150 : -150;
const dragByX = config.direction === 'rtl' ? DRAG_DISTANCE_MULTIPLE_OPTIONS : -DRAG_DISTANCE_MULTIPLE_OPTIONS;

await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
await page.waitForChanges();

await expect(item).toHaveScreenshot(screenshot('item-sliding-end'));
});
Expand All @@ -61,7 +68,16 @@ configs({ modes: ['md'], directions: ['ltr'] }).forEach(({ title, screenshot, co
await page.goto(`/src/components/item-sliding/test/basic`, config);
const item = page.locator('#item2');

await dragElementBy(item, page, -150, 0, undefined, undefined, true, 20);
await dragElementBy(
item,
page,
-DRAG_DISTANCE_MULTIPLE_OPTIONS,
0,
undefined,
undefined,
true,
DRAG_STEPS_UNDER_FULL_SWIPE
);
await page.waitForChanges();

// item-sliding doesn't have an easy way to tell whether it's fully open so just screenshot it
Expand Down Expand Up @@ -140,8 +156,8 @@ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, conf
const direction = config.direction;
const item = page.locator('ion-item-sliding');

const dragByX = direction == 'rtl' ? -150 : 150;
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
const dragByX = direction == 'rtl' ? -DRAG_DISTANCE_MULTIPLE_OPTIONS : DRAG_DISTANCE_MULTIPLE_OPTIONS;
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
await page.waitForChanges();

await expect(item).toHaveScreenshot(screenshot(`item-sliding-safe-area-left`));
Expand Down Expand Up @@ -181,8 +197,8 @@ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, conf
const direction = config.direction;
const item = page.locator('ion-item-sliding');

const dragByX = direction == 'rtl' ? 150 : -150;
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
const dragByX = direction == 'rtl' ? DRAG_DISTANCE_MULTIPLE_OPTIONS : -DRAG_DISTANCE_MULTIPLE_OPTIONS;
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
await page.waitForChanges();

await expect(item).toHaveScreenshot(screenshot(`item-sliding-safe-area-right`));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'], directions: ['ltr', 'rtl'] }).forEac
<ion-item-option expandable="true">Delete</ion-item-option>
</ion-item-options>
</ion-item-sliding>
`,
`,
config
);

Expand All @@ -52,7 +52,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'], directions: ['ltr', 'rtl'] }).forEac
<ion-item-option expandable="true">Archive</ion-item-option>
</ion-item-options>
</ion-item-sliding>
`,
`,
config
);

Expand All @@ -77,7 +77,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'], directions: ['ltr', 'rtl'] }).forEac
<ion-item-option expandable="true">Delete</ion-item-option>
</ion-item-options>
</ion-item-sliding>
`,
`,
config
);

Expand Down Expand Up @@ -132,7 +132,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'], directions: ['ltr', 'rtl'] }).forEac
<ion-item-option>Edit</ion-item-option>
</ion-item-options>
</ion-item-sliding>
`,
`,
config
);

Expand Down Expand Up @@ -166,7 +166,7 @@ configs({ modes: ['md'], directions: ['ltr', 'rtl'] }).forEach(({ title, config
<ion-item-option expandable="true">Delete</ion-item-option>
</ion-item-options>
</ion-item-sliding>
`,
`,
config
);

Expand Down
21 changes: 12 additions & 9 deletions core/src/components/item-sliding/test/icons/item-sliding.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect } from '@playwright/test';
import { configs, test, dragElementBy } from '@utils/test/playwright';

import { DRAG_DISTANCE_MULTIPLE_OPTIONS, DRAG_STEPS_UNDER_FULL_SWIPE } from '../test.utils';

/**
* item-sliding doesn't have mode-specific styling,
* but the child components, item-options and item-option, do.
Expand All @@ -10,28 +12,29 @@ import { configs, test, dragElementBy } from '@utils/test/playwright';
*/
configs({ modes: ['ionic-md', 'ios', 'md'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('item-sliding: icons'), () => {
test('should not have visual regressions', async ({ page }) => {
test.beforeEach(async ({ page }) => {
await page.goto(`/src/components/item-sliding/test/icons`, config);
});

const itemIDs = ['iconsOnly', 'iconsStart', 'iconsEnd', 'iconsTop', 'iconsBottom'];
for (const itemID of itemIDs) {
const item = page.locator(`#${itemID}`);
['iconsOnly', 'iconsStart', 'iconsEnd', 'iconsTop', 'iconsBottom'].forEach((position) => {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Big upgrade from the old for loop! Nice work!

test(`${position} - should not have visual regressions`, async ({ page }) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By breaking it apart, it's easier to determine which test failed.

const item = page.locator(`#${position}`);

/**
* Negative dragByX value to drag element from the right to the left
* to reveal the options on the right side.
* Positive dragByX value to drag element from the left to the right
* to reveal the options on the left side.
*/
const dragByX = config.direction === 'rtl' ? 150 : -150;
const dragByX = config.direction === 'rtl' ? DRAG_DISTANCE_MULTIPLE_OPTIONS : -DRAG_DISTANCE_MULTIPLE_OPTIONS;

await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
await page.waitForChanges();

// Convert camelCase ids to kebab-case for screenshot file names
const itemIDKebab = itemID.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
await expect(item).toHaveScreenshot(screenshot(`item-sliding-${itemIDKebab}`));
}
const positionKebab = position.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
await expect(item).toHaveScreenshot(screenshot(`item-sliding-${positionKebab}`));
});
});
});
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { expect } from '@playwright/test';
import { configs, test, dragElementBy } from '@utils/test/playwright';

import { DRAG_DISTANCE_MULTIPLE_OPTIONS } from '../test.utils';

/**
* This behavior does not vary across modes/directions
*/
Expand All @@ -15,7 +18,15 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>

expect(await scrollEl.evaluate((el: HTMLElement) => el.scrollTop)).toEqual(0);

await dragElementBy(itemSlidingEl, page, -150, 0, undefined, undefined, false);
/**
* No need to increase steps to prevent the full swipe threshold
* from being crossed because:
* - we are not testing swipe behavior here
* - increasing steps is only in Webkit since it accumulates velocity
* faster than other browsers, and this test is skipped in Webkit, so
* the default step count is safe to use
*/
await dragElementBy(itemSlidingEl, page, -DRAG_DISTANCE_MULTIPLE_OPTIONS, 0, undefined, undefined, false);

/**
* Do not use scrollToBottom() or other scrolling methods
Expand Down
22 changes: 11 additions & 11 deletions core/src/components/item-sliding/test/shapes/item-sliding.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,33 +1,33 @@
import { expect } from '@playwright/test';
import { configs, test, dragElementBy } from '@utils/test/playwright';

import { DRAG_DISTANCE_MULTIPLE_OPTIONS, DRAG_STEPS_UNDER_FULL_SWIPE } from '../test.utils';

/**
* The shapes on the `item-option` do not vary by direction
* when they are not being dragged.
*/
configs({ modes: ['ionic-md'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
test.describe(title('item-sliding: shapes'), () => {
test('should not have visual regressions when not expanded', async ({ page, skip }) => {
test.beforeEach(async ({ page }) => {
await page.goto(`/src/components/item-sliding/test/shapes`, config);
});

// TODO(FW-7288): Remove skip once fix has been implemented
skip.browser('webkit', 'Flaky test in Safari');

const itemIDs = ['round', 'soft', 'rectangular'];
for (const itemID of itemIDs) {
const item = page.locator(`#${itemID}`);
['round', 'soft', 'rectangular'].forEach((shape) => {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By breaking it apart, it's easier to determine which test failed.

test(`${shape} - should not have visual regressions when not expanded`, async ({ page }) => {
const item = page.locator(`#${shape}`);

/**
* Negative dragByX value to drag element from the right to the left
* to reveal the options on the right side.
*/
const dragByX = -150;
const dragByX = -DRAG_DISTANCE_MULTIPLE_OPTIONS;

await dragElementBy(item, page, dragByX);
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
await page.waitForChanges();

await expect(item).toHaveScreenshot(screenshot(`item-sliding-${itemID}`));
}
await expect(item).toHaveScreenshot(screenshot(`item-sliding-${shape}`));
});
});
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { expect } from '@playwright/test';
import { configs, test, dragElementBy } from '@utils/test/playwright';

import { DRAG_DISTANCE_MULTIPLE_OPTIONS } from '../test.utils';

/**
* This behavior does not vary across modes
*/
Expand All @@ -16,8 +18,13 @@ configs({ modes: ['ionic-md', 'md', 'ios'], directions: ['ltr'] }).forEach(({ ti
* Negative dragByX value to drag element from the right to the left
* to reveal the options on the right side.
*/
const dragByX = -150;
const dragByX = -DRAG_DISTANCE_MULTIPLE_OPTIONS;

/**
* No need to increase steps to prevent the full swipe threshold from
* being crossed because the option is disabled, so the option will
* never expand fully regardless of drag speed.
*/
await dragElementBy(item, page, dragByX);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still uses the default steps (10), but the equivalent 150px drags in basic, icons, and shapes all pass steps=15. scroll-target is in the same boat. Is disabled-options exempt from the full-swipe threshold (and is releaseDrag=false enough for scroll-target), or did these get missed? A one-liner about why would help the next person figure out which value to use.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the reason why as comments: c763b6a

await page.waitForChanges();

Expand Down
15 changes: 15 additions & 0 deletions core/src/components/item-sliding/test/test.utils.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,21 @@
import { expect } from '@playwright/test';
import type { E2EPage, ScreenshotFn } from '@utils/test/playwright';

/**
* Drag distances that reveal options without crossing the full swipe
* threshold (`optsWidth` + `SWIPE_MARGIN`). A narrower options panel
* requires a shorter drag.
*/
export const DRAG_DISTANCE_SINGLE_OPTION = 100;
export const DRAG_DISTANCE_MULTIPLE_OPTIONS = 150;

/**
* The number of drag steps used when revealing options. A higher step
* count slows the drag velocity, keeping it below the full swipe
* threshold in WebKit. See `dragElementBy` for more details.
*/
export const DRAG_STEPS_UNDER_FULL_SWIPE = 15;

/**
* Warning: This function will fail when in RTL mode.
* TODO(FW-3711): Remove the `directions` config when this issue preventing
Expand Down
34 changes: 29 additions & 5 deletions core/src/utils/test/playwright/drag-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ import type { ElementHandle, Locator } from '@playwright/test';

import type { E2EPage } from './';

/**
* Drags an element by the given number of pixels on the X and Y axes.
*
* @param el The element to drag.
* @param page The E2E Page object.
* @param dragByX The number of pixels to drag on the X axis. Negative values drag left, positive values drag right.
* @param dragByY The number of pixels to drag on the Y axis. Negative values drag up, positive values drag down.
* @param startXCoord The X coordinate to start the drag from. Defaults to the center of the element.
* @param startYCoord The Y coordinate to start the drag from. Defaults to the center of the element.
* @param releaseDrag Whether to release the drag at the end of the gesture. Defaults to `true`.
* @param steps The number of steps to divide the drag into. More steps reduce velocity; fewer steps increase it. Use this to control whether velocity-based thresholds (e.g. full-swipe) are triggered, particularly in Safari where gesture velocity is calculated relative to animation frames. Defaults to `10`.
*/
export const dragElementBy = async (
el: Locator | ElementHandle<SVGElement | HTMLElement>,
page: E2EPage,
Expand Down Expand Up @@ -46,10 +58,11 @@ export const dragElementBy = async (

/**
* Drags an element by the given amount of pixels on the Y axis.
*
* @param el The element to drag.
* @param page The E2E Page object.
* @param dragByY The amount of pixels to drag the element by.
* @param startYCoord The Y coordinate to start the drag gesture at. Defaults to the center of the element.
* @param dragByY The number of pixels to drag on the Y axis.
* @param startYCoord The Y coordinate to start the drag from. Defaults to the center of the element.
*/
export const dragElementByYAxis = async (
el: Locator | ElementHandle<SVGElement | HTMLElement>,
Expand Down Expand Up @@ -139,10 +152,21 @@ const moveElement = async (page: E2EPage, startX: number, startY: number, dragBy

await page.mouse.move(middleX, middleY);

// Safari needs to wait for a repaint to occur before moving the mouse again.
/**
* In Safari, gesture velocity is calculated relative to animation
* frames, causing velocity to accumulate faster than in other
* browsers. Without waiting for a repaint, consecutive `mouse.move`
* events arrive with ~0ms time delta and velocity never accumulates,
* causing gesture
* detection to fail.
*/
if (browser === 'webkit' && i % 2 === 0) {
// Repainting every 2 steps is enough to keep the drag gesture smooth.
// Anything past 4 steps will cause the drag gesture to be flaky.
/**
* Repainting every 2 steps is enough to keep the drag gesture
* smooth. Repainting on every step makes the test slow, and
* repainting every 4+ steps means Safari does not see enough
* frames to track the gesture reliably.
*/
await page.evaluate(() => new Promise(requestAnimationFrame));
}
}
Expand Down
Loading