Skip to content

Commit c763b6a

Browse files
thetaPCthetaPC
committed
docs(item-sliding): add why steps were not increased
1 parent f7df147 commit c763b6a

7 files changed

Lines changed: 53 additions & 18 deletions

File tree

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

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

4-
import { DRAG_DISTANCE_SINGLE_OPTION, DRAG_DISTANCE_MULTIPLE_OPTIONS } from '../test.utils';
4+
import {
5+
DRAG_DISTANCE_SINGLE_OPTION,
6+
DRAG_DISTANCE_MULTIPLE_OPTIONS,
7+
DRAG_STEPS_UNDER_FULL_SWIPE,
8+
} from '../test.utils';
59

610
/**
711
* item-sliding doesn't have mode-specific styling,
@@ -27,7 +31,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, conf
2731
*/
2832
const dragByX = config.direction === 'rtl' ? -DRAG_DISTANCE_SINGLE_OPTION : DRAG_DISTANCE_SINGLE_OPTION;
2933

30-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 15);
34+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
3135

3236
await expect(item).toHaveScreenshot(screenshot('item-sliding-start'));
3337
});
@@ -45,7 +49,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, conf
4549
*/
4650
const dragByX = config.direction === 'rtl' ? DRAG_DISTANCE_MULTIPLE_OPTIONS : -DRAG_DISTANCE_MULTIPLE_OPTIONS;
4751

48-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 15);
52+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
4953

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

65-
await dragElementBy(item, page, -DRAG_DISTANCE_MULTIPLE_OPTIONS, 0, undefined, undefined, true, 15);
69+
await dragElementBy(
70+
item,
71+
page,
72+
-DRAG_DISTANCE_MULTIPLE_OPTIONS,
73+
0,
74+
undefined,
75+
undefined,
76+
true,
77+
DRAG_STEPS_UNDER_FULL_SWIPE
78+
);
6679
await page.waitForChanges();
6780

6881
// item-sliding doesn't have an easy way to tell whether it's fully open so just screenshot it
@@ -142,7 +155,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, conf
142155
const item = page.locator('ion-item-sliding');
143156

144157
const dragByX = direction == 'rtl' ? -DRAG_DISTANCE_MULTIPLE_OPTIONS : DRAG_DISTANCE_MULTIPLE_OPTIONS;
145-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 15);
158+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
146159
await page.waitForChanges();
147160

148161
await expect(item).toHaveScreenshot(screenshot(`item-sliding-safe-area-left`));
@@ -183,7 +196,7 @@ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, conf
183196
const item = page.locator('ion-item-sliding');
184197

185198
const dragByX = direction == 'rtl' ? DRAG_DISTANCE_MULTIPLE_OPTIONS : -DRAG_DISTANCE_MULTIPLE_OPTIONS;
186-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 15);
199+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
187200
await page.waitForChanges();
188201

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

‎core/src/components/item-sliding/test/icons/item-sliding.e2e.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
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';
4+
import { DRAG_DISTANCE_MULTIPLE_OPTIONS, DRAG_STEPS_UNDER_FULL_SWIPE } from '../test.utils';
55

66
/**
77
* item-sliding doesn't have mode-specific styling,
@@ -28,7 +28,7 @@ configs({ modes: ['ionic-md', 'ios', 'md'] }).forEach(({ title, screenshot, conf
2828
*/
2929
const dragByX = config.direction === 'rtl' ? DRAG_DISTANCE_MULTIPLE_OPTIONS : -DRAG_DISTANCE_MULTIPLE_OPTIONS;
3030

31-
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, 15);
31+
await dragElementBy(item, page, dragByX, 0, undefined, undefined, true, DRAG_STEPS_UNDER_FULL_SWIPE);
3232
await page.waitForChanges();
3333

3434
// Convert camelCase ids to kebab-case for screenshot file names

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,14 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
1818

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

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+
*/
2129
await dragElementBy(itemSlidingEl, page, -DRAG_DISTANCE_MULTIPLE_OPTIONS, 0, undefined, undefined, false);
2230

2331
/**

‎core/src/components/item-sliding/test/shapes/item-sliding.e2e.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
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';
4+
import { DRAG_DISTANCE_MULTIPLE_OPTIONS, DRAG_STEPS_UNDER_FULL_SWIPE } from '../test.utils';
55

66
/**
77
* The shapes on the `item-option` do not vary by direction
@@ -23,7 +23,7 @@ configs({ modes: ['ionic-md'], directions: ['ltr'] }).forEach(({ title, screensh
2323
*/
2424
const dragByX = -DRAG_DISTANCE_MULTIPLE_OPTIONS;
2525

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

2929
await expect(item).toHaveScreenshot(screenshot(`item-sliding-${shape}`));

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,11 @@ configs({ modes: ['ionic-md', 'md', 'ios'], directions: ['ltr'] }).forEach(({ ti
2020
*/
2121
const dragByX = -DRAG_DISTANCE_MULTIPLE_OPTIONS;
2222

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+
*/
2328
await dragElementBy(item, page, dragByX);
2429
await page.waitForChanges();
2530

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,19 @@ import type { E2EPage, ScreenshotFn } from '@utils/test/playwright';
33

44
/**
55
* Drag distances that reveal options without crossing the full swipe
6-
* threshold (optsWidth + SWIPE_MARGIN). A narrower options panel
6+
* threshold (`optsWidth` + `SWIPE_MARGIN`). A narrower options panel
77
* requires a shorter drag.
88
*/
99
export const DRAG_DISTANCE_SINGLE_OPTION = 100;
1010
export const DRAG_DISTANCE_MULTIPLE_OPTIONS = 150;
1111

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+
1219
/**
1320
* Warning: This function will fail when in RTL mode.
1421
* TODO(FW-3711): Remove the `directions` config when this issue preventing

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

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -153,17 +153,19 @@ const moveElement = async (page: E2EPage, startX: number, startY: number, dragBy
153153
await page.mouse.move(middleX, middleY);
154154

155155
/**
156-
* In Safari, gesture velocity is calculated relative to animation frames.
157-
* Without waiting for a repaint, consecutive `mouse.move` events arrive
158-
* with ~0ms time delta and velocity never accumulates, causing gesture
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
159161
* detection to fail.
160162
*/
161163
if (browser === 'webkit' && i % 2 === 0) {
162164
/**
163-
* Repainting every 2 steps is enough to keep the drag gesture smooth.
164-
* Repainting on every step makes the test slow, and repainting every
165-
* 4+ steps means Safari does not see enough frames to track the gesture
166-
* reliably.
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.
167169
*/
168170
await page.evaluate(() => new Promise(requestAnimationFrame));
169171
}

0 commit comments

Comments
 (0)