Skip to content

Commit d1ad1ad

Browse files
thetaPCShaneK
andauthored
test(drag-element, item-sliding): prevent full swipe (#31097)
Issue number: resolves internal Co-authored-by: Shane <shane@shanessite.net>
1 parent 08f05b3 commit d1ad1ad

9 files changed

Lines changed: 117 additions & 41 deletions

File tree

core/src/components/item-sliding/test/basic/item-sliding.e2e.ts

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { expect } from '@playwright/test';
22
import { configs, dragElementBy, test } from '@utils/test/playwright';
33

4+
import {
5+
DRAG_DISTANCE_SINGLE_OPTION,
6+
DRAG_DISTANCE_MULTIPLE_OPTIONS,
7+
DRAG_STEPS_UNDER_FULL_SWIPE,
8+
} from '../test.utils';
9+
410
/**
511
* item-sliding doesn't have mode-specific styling,
612
* but the child components, item-options and item-option, do.
@@ -23,9 +29,9 @@ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, conf
2329
* Positive dragByX value to drag element from the left to the right
2430
* to reveal the options on the left side.
2531
*/
26-
const dragByX = config.direction === 'rtl' ? -100 : 100;
32+
const dragByX = config.direction === 'rtl' ? -DRAG_DISTANCE_SINGLE_OPTION : DRAG_DISTANCE_SINGLE_OPTION;
2733

28-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
34+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
2935
await page.waitForChanges();
3036

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

47-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
53+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
54+
await page.waitForChanges();
4855

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

64-
await dragElementBy(item, page, -150, 0, undefined, undefined, true, 20);
71+
await dragElementBy(
72+
item,
73+
page,
74+
-DRAG_DISTANCE_MULTIPLE_OPTIONS,
75+
0,
76+
undefined,
77+
undefined,
78+
true,
79+
DRAG_STEPS_UNDER_FULL_SWIPE
80+
);
6581
await page.waitForChanges();
6682

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

143-
const dragByX = direction == 'rtl' ? -150 : 150;
144-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
159+
const dragByX = direction == 'rtl' ? -DRAG_DISTANCE_MULTIPLE_OPTIONS : DRAG_DISTANCE_MULTIPLE_OPTIONS;
160+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
145161
await page.waitForChanges();
146162

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

184-
const dragByX = direction == 'rtl' ? 150 : -150;
185-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
200+
const dragByX = direction == 'rtl' ? DRAG_DISTANCE_MULTIPLE_OPTIONS : -DRAG_DISTANCE_MULTIPLE_OPTIONS;
201+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
186202
await page.waitForChanges();
187203

188204
await expect(item).toHaveScreenshot(screenshot(`item-sliding-safe-area-right`));

core/src/components/item-sliding/test/full-swipe/item-sliding.e2e.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'], directions: ['ltr', 'rtl'] }).forEac
2727
<ion-item-option expandable="true">Delete</ion-item-option>
2828
</ion-item-options>
2929
</ion-item-sliding>
30-
`,
30+
`,
3131
config
3232
);
3333

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

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

@@ -132,7 +132,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'], directions: ['ltr', 'rtl'] }).forEac
132132
<ion-item-option>Edit</ion-item-option>
133133
</ion-item-options>
134134
</ion-item-sliding>
135-
`,
135+
`,
136136
config
137137
);
138138

@@ -166,7 +166,7 @@ configs({ modes: ['md'], directions: ['ltr', 'rtl'] }).forEach(({ title, config
166166
<ion-item-option expandable="true">Delete</ion-item-option>
167167
</ion-item-options>
168168
</ion-item-sliding>
169-
`,
169+
`,
170170
config
171171
);
172172

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { expect } from '@playwright/test';
22
import { configs, test, dragElementBy } from '@utils/test/playwright';
33

4+
import { DRAG_DISTANCE_MULTIPLE_OPTIONS, DRAG_STEPS_UNDER_FULL_SWIPE } from '../test.utils';
5+
46
/**
57
* item-sliding doesn't have mode-specific styling,
68
* but the child components, item-options and item-option, do.
@@ -10,28 +12,29 @@ import { configs, test, dragElementBy } from '@utils/test/playwright';
1012
*/
1113
configs({ modes: ['ionic-md', 'ios', 'md'] }).forEach(({ title, screenshot, config }) => {
1214
test.describe(title('item-sliding: icons'), () => {
13-
test('should not have visual regressions', async ({ page }) => {
15+
test.beforeEach(async ({ page }) => {
1416
await page.goto(`/src/components/item-sliding/test/icons`, config);
17+
});
1518

16-
const itemIDs = ['iconsOnly', 'iconsStart', 'iconsEnd', 'iconsTop', 'iconsBottom'];
17-
for (const itemID of itemIDs) {
18-
const item = page.locator(`#${itemID}`);
19+
['iconsOnly', 'iconsStart', 'iconsEnd', 'iconsTop', 'iconsBottom'].forEach((position) => {
20+
test(`${position} - should not have visual regressions`, async ({ page }) => {
21+
const item = page.locator(`#${position}`);
1922

2023
/**
2124
* Negative dragByX value to drag element from the right to the left
2225
* to reveal the options on the right side.
2326
* Positive dragByX value to drag element from the left to the right
2427
* to reveal the options on the left side.
2528
*/
26-
const dragByX = config.direction === 'rtl' ? 150 : -150;
29+
const dragByX = config.direction === 'rtl' ? DRAG_DISTANCE_MULTIPLE_OPTIONS : -DRAG_DISTANCE_MULTIPLE_OPTIONS;
2730

28-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 20);
31+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
2932
await page.waitForChanges();
3033

3134
// Convert camelCase ids to kebab-case for screenshot file names
32-
const itemIDKebab = itemID.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
33-
await expect(item).toHaveScreenshot(screenshot(`item-sliding-${itemIDKebab}`));
34-
}
35+
const positionKebab = position.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase();
36+
await expect(item).toHaveScreenshot(screenshot(`item-sliding-${positionKebab}`));
37+
});
3538
});
3639
});
3740
});

core/src/components/item-sliding/test/scroll-target/item-sliding.e2e.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { expect } from '@playwright/test';
22
import { configs, test, dragElementBy } from '@utils/test/playwright';
3+
4+
import { DRAG_DISTANCE_MULTIPLE_OPTIONS } from '../test.utils';
5+
36
/**
47
* This behavior does not vary across modes/directions
58
*/
@@ -15,7 +18,15 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
1518

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

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

2031
/**
2132
* Do not use scrollToBottom() or other scrolling methods
Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,33 @@
11
import { expect } from '@playwright/test';
22
import { configs, test, dragElementBy } from '@utils/test/playwright';
33

4+
import { DRAG_DISTANCE_MULTIPLE_OPTIONS, DRAG_STEPS_UNDER_FULL_SWIPE } from '../test.utils';
5+
46
/**
57
* The shapes on the `item-option` do not vary by direction
68
* when they are not being dragged.
79
*/
810
configs({ modes: ['ionic-md'], directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
911
test.describe(title('item-sliding: shapes'), () => {
10-
test('should not have visual regressions when not expanded', async ({ page, skip }) => {
12+
test.beforeEach(async ({ page }) => {
1113
await page.goto(`/src/components/item-sliding/test/shapes`, config);
14+
});
1215

13-
// TODO(FW-7288): Remove skip once fix has been implemented
14-
skip.browser('webkit', 'Flaky test in Safari');
15-
16-
const itemIDs = ['round', 'soft', 'rectangular'];
17-
for (const itemID of itemIDs) {
18-
const item = page.locator(`#${itemID}`);
16+
['round', 'soft', 'rectangular'].forEach((shape) => {
17+
test(`${shape} - should not have visual regressions when not expanded`, async ({ page }) => {
18+
const item = page.locator(`#${shape}`);
1919

2020
/**
2121
* Negative dragByX value to drag element from the right to the left
2222
* to reveal the options on the right side.
2323
*/
24-
const dragByX = -150;
24+
const dragByX = -DRAG_DISTANCE_MULTIPLE_OPTIONS;
2525

26-
await dragElementBy(item, page, dragByX);
26+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
2727
await page.waitForChanges();
2828

29-
await expect(item).toHaveScreenshot(screenshot(`item-sliding-${itemID}`));
30-
}
29+
await expect(item).toHaveScreenshot(screenshot(`item-sliding-${shape}`));
30+
});
3131
});
3232
});
3333
});
Loading

core/src/components/item-sliding/test/states/item-sliding.e2e.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { expect } from '@playwright/test';
22
import { configs, test, dragElementBy } from '@utils/test/playwright';
33

4+
import { DRAG_DISTANCE_MULTIPLE_OPTIONS } from '../test.utils';
5+
46
/**
57
* This behavior does not vary across modes
68
*/
@@ -16,8 +18,13 @@ configs({ modes: ['ionic-md', 'md', 'ios'], directions: ['ltr'] }).forEach(({ ti
1618
* Negative dragByX value to drag element from the right to the left
1719
* to reveal the options on the right side.
1820
*/
19-
const dragByX = -150;
21+
const dragByX = -DRAG_DISTANCE_MULTIPLE_OPTIONS;
2022

23+
/**
24+
* No need to increase steps to prevent the full swipe threshold from
25+
* being crossed because the option is disabled, so the option will
26+
* never expand fully regardless of drag speed.
27+
*/
2128
await dragElementBy(item, page, dragByX);
2229
await page.waitForChanges();
2330

core/src/components/item-sliding/test/test.utils.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,21 @@
11
import { expect } from '@playwright/test';
22
import type { E2EPage, ScreenshotFn } from '@utils/test/playwright';
33

4+
/**
5+
* Drag distances that reveal options without crossing the full swipe
6+
* threshold (`optsWidth` + `SWIPE_MARGIN`). A narrower options panel
7+
* requires a shorter drag.
8+
*/
9+
export const DRAG_DISTANCE_SINGLE_OPTION = 100;
10+
export const DRAG_DISTANCE_MULTIPLE_OPTIONS = 150;
11+
12+
/**
13+
* The number of drag steps used when revealing options. A higher step
14+
* count slows the drag velocity, keeping it below the full swipe
15+
* threshold in WebKit. See `dragElementBy` for more details.
16+
*/
17+
export const DRAG_STEPS_UNDER_FULL_SWIPE = 15;
18+
419
/**
520
* Warning: This function will fail when in RTL mode.
621
* TODO(FW-3711): Remove the `directions` config when this issue preventing

core/src/utils/test/playwright/drag-element.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,18 @@ import type { ElementHandle, Locator } from '@playwright/test';
1010

1111
import type { E2EPage } from './';
1212

13+
/**
14+
* Drags an element by the given number of pixels on the X and Y axes.
15+
*
16+
* @param el The element to drag.
17+
* @param page The E2E Page object.
18+
* @param dragByX The number of pixels to drag on the X axis. Negative values drag left, positive values drag right.
19+
* @param dragByY The number of pixels to drag on the Y axis. Negative values drag up, positive values drag down.
20+
* @param startXCoord The X coordinate to start the drag from. Defaults to the center of the element.
21+
* @param startYCoord The Y coordinate to start the drag from. Defaults to the center of the element.
22+
* @param releaseDrag Whether to release the drag at the end of the gesture. Defaults to `true`.
23+
* @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`.
24+
*/
1325
export const dragElementBy = async (
1426
el: Locator | ElementHandle<SVGElement | HTMLElement>,
1527
page: E2EPage,
@@ -46,10 +58,11 @@ export const dragElementBy = async (
4658

4759
/**
4860
* Drags an element by the given amount of pixels on the Y axis.
61+
*
4962
* @param el The element to drag.
5063
* @param page The E2E Page object.
51-
* @param dragByY The amount of pixels to drag the element by.
52-
* @param startYCoord The Y coordinate to start the drag gesture at. Defaults to the center of the element.
64+
* @param dragByY The number of pixels to drag on the Y axis.
65+
* @param startYCoord The Y coordinate to start the drag from. Defaults to the center of the element.
5366
*/
5467
export const dragElementByYAxis = async (
5568
el: Locator | ElementHandle<SVGElement | HTMLElement>,
@@ -139,10 +152,21 @@ const moveElement = async (page: E2EPage, startX: number, startY: number, dragBy
139152

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

142-
// Safari needs to wait for a repaint to occur before moving the mouse again.
155+
/**
156+
* In Safari, gesture velocity is calculated relative to animation
157+
* frames, causing velocity to accumulate faster than in other
158+
* browsers. Without waiting for a repaint, consecutive `mouse.move`
159+
* events arrive with ~0ms time delta and velocity never accumulates,
160+
* causing gesture
161+
* detection to fail.
162+
*/
143163
if (browser === 'webkit' && i % 2 === 0) {
144-
// Repainting every 2 steps is enough to keep the drag gesture smooth.
145-
// Anything past 4 steps will cause the drag gesture to be flaky.
164+
/**
165+
* Repainting every 2 steps is enough to keep the drag gesture
166+
* smooth. Repainting on every step makes the test slow, and
167+
* repainting every 4+ steps means Safari does not see enough
168+
* frames to track the gesture reliably.
169+
*/
146170
await page.evaluate(() => new Promise(requestAnimationFrame));
147171
}
148172
}

0 commit comments

Comments
 (0)