Skip to content

fix(item-sliding): swiping inside of virtual scroller now prevents scrolling #25345

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
May 31, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 11 additions & 23 deletions core/src/components/item-sliding/item-sliding.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { Component, Element, Event, Host, Method, Prop, State, Watch, h } from '

import { getIonMode } from '../../global/ionic-global';
import type { Gesture, GestureDetail, Side } from '../../interface';
import { findClosestIonContent, disableContentScrollY, resetContentScrollY } from '../../utils/content';
import { isEndSide } from '../../utils/helpers';

const SWIPE_MARGIN = 30;
Expand Down Expand Up @@ -43,7 +44,7 @@ export class ItemSliding implements ComponentInterface {
private rightOptions?: HTMLIonItemOptionsElement;
private optsDirty = true;
private gesture?: Gesture;
private closestContent: HTMLIonContentElement | null = null;
private contentEl: HTMLElement | null = null;
private initialContentScrollY = true;

@Element() el!: HTMLIonItemSlidingElement;
Expand All @@ -68,7 +69,7 @@ export class ItemSliding implements ComponentInterface {

async connectedCallback() {
this.item = this.el.querySelector('ion-item');
this.closestContent = this.el.closest('ion-content');
this.contentEl = findClosestIonContent(this.el);

await this.updateOptions();

Expand Down Expand Up @@ -264,23 +265,6 @@ export class ItemSliding implements ComponentInterface {
return !!(this.rightOptions || this.leftOptions);
}

private disableContentScrollY() {
if (this.closestContent === null) {
return;
}

this.initialContentScrollY = this.closestContent.scrollY;
this.closestContent.scrollY = false;
}

private restoreContentScrollY() {
if (this.closestContent === null) {
return;
}

this.closestContent.scrollY = this.initialContentScrollY;
}

private onStart() {
/**
* We need to query for the ion-item
Expand All @@ -289,8 +273,10 @@ export class ItemSliding implements ComponentInterface {
*/
this.item = this.el.querySelector('ion-item');

// Prevent scrolling during gesture
this.disableContentScrollY();
const { contentEl } = this;
if (contentEl) {
this.initialContentScrollY = disableContentScrollY(contentEl);
}

openSlidingItem = this.el;

Expand Down Expand Up @@ -343,8 +329,10 @@ export class ItemSliding implements ComponentInterface {
}

private onEnd(gesture: GestureDetail) {
// Restore ion-content scrollY to initial value when gesture ends
this.restoreContentScrollY();
const { contentEl, initialContentScrollY } = this;
if (contentEl) {
resetContentScrollY(contentEl, initialContentScrollY);
}

const velocity = gesture.velocityX;

Expand Down
4 changes: 2 additions & 2 deletions core/src/components/item-sliding/test/basic/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ <h2>No Options</h2>
</ion-item-options>
</ion-item-sliding>

<ion-item-sliding id="item6">
<ion-item-sliding id="two-options">
<ion-item>
<ion-label> Two options, one dynamic option and text </ion-label>
</ion-item>
Expand All @@ -89,7 +89,7 @@ <h2>No Options</h2>
<ion-icon slot="start" ios="ellipsis-horizontal" md="ellipsis-vertical"></ion-icon>
<span class="more-text"></span>
</ion-item-option>
<ion-item-option color="secondary" onclick="archive('item6')">
<ion-item-option color="secondary" onclick="archive('two-options')">
<ion-icon slot="start" name="archive"></ion-icon>
<span class="archive-text"></span>
</ion-item-option>
Expand Down
35 changes: 35 additions & 0 deletions core/src/components/item-sliding/test/basic/item-sliding.e2e.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import { expect } from '@playwright/test';
import { test } from '@utils/test/playwright';

test.describe('item-sliding: basic', () => {
test('should not scroll when the item-sliding is swiped', async ({ page, browserName }, testInfo) => {
test.skip(browserName === 'webkit', 'mouse.wheel is not available in WebKit');
test.skip(testInfo.project.metadata.rtl === true, 'This feature does not have RTL-specific behaviors');

await page.goto(`/src/components/item-sliding/test/basic`);

const itemSlidingEl = page.locator('#two-options');
const scrollEl = page.locator('ion-content .inner-scroll');

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

const box = (await itemSlidingEl.boundingBox())!;
const centerX = box.x + box.width / 2;
const centerY = box.y + box.height / 2;

await page.mouse.move(centerX, centerY);
await page.mouse.down();
await page.mouse.move(centerX - 30, centerY);

/**
* Do not use scrollToBottom() or other scrolling methods
* on ion-content as those will update the scroll position.
* Setting scrollTop still works even with overflow-y: hidden.
* However, simulating a user gesture should not scroll the content.
*/
await page.mouse.wheel(0, 100);
await page.waitForChanges();

expect(await scrollEl.evaluate((el: HTMLElement) => el.scrollTop)).toEqual(0);
});
});
89 changes: 89 additions & 0 deletions core/src/components/item-sliding/test/scroll-target/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
<!DOCTYPE html>
<html lang="en" dir="ltr">
<head>
<meta charset="UTF-8" />
<title>Item Sliding - Scroll Target</title>
<meta
name="viewport"
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=1.0, user-scalable=no"
/>
<link href="../../../../../css/ionic.bundle.css" rel="stylesheet" />
<link href="../../../../../scripts/testing/styles.css" rel="stylesheet" />
<script src="../../../../../scripts/testing/scripts.js"></script>
<script nomodule src="../../../../../dist/ionic/ionic.js"></script>
<script type="module" src="../../../../../dist/ionic/ionic.esm.js"></script>
<style>
.ion-content-scroll-host {
width: 100%;
height: 100%;

overflow-y: scroll;
}
</style>
</head>
<body>
<ion-app>
<ion-header>
<ion-toolbar>
<ion-title>Item Sliding - Scroll Target</ion-title>
</ion-toolbar>
</ion-header>

<ion-content class="ion-padding" scroll-y="false">
<div class="ion-content-scroll-host">
<p>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper.
Nam nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat
libero id, feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl
convallis maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget
lobortis finibus, lectus lorem maximus purus, quis sagittis tortor sem sed tellus.
</p>

<ion-item-sliding>
<ion-item>
<ion-label>Item Sliding</ion-label>
</ion-item>
<ion-item-options side="end" class="show-options">
<ion-item-option color="primary">
<ion-icon slot="start" ios="ellipsis-horizontal" md="ellipsis-vertical"></ion-icon>
<span class="more-text"></span>
</ion-item-option>
</ion-item-options>
</ion-item-sliding>

<p>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper.
Nam nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat
libero id, feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl
convallis maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget
lobortis finibus, lectus lorem maximus purus, quis sagittis tortor sem sed tellus.
</p>

<p>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper.
Nam nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat
libero id, feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl
convallis maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget
lobortis finibus, lectus lorem maximus purus, quis sagittis tortor sem sed tellus.
</p>

<p>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper.
Nam nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat
libero id, feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl
convallis maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget
lobortis finibus, lectus lorem maximus purus, quis sagittis tortor sem sed tellus.
</p>

<p>
Lorem ipsum dolor sit amet, consectetur adipiscing elit. Curabitur faucibus nulla a nunc tincidunt semper.
Nam nibh lorem, pharetra ac ex ac, tempus fringilla est. Aenean tincidunt ipsum pellentesque, consequat
libero id, feugiat leo. In vestibulum faucibus velit, non tincidunt erat tincidunt in. Donec a diam sed nisl
convallis maximus. Aenean cursus sagittis lorem vitae tristique. Pellentesque pellentesque, quam eget
lobortis finibus, lectus lorem maximus purus, quis sagittis tortor sem sed tellus.
</p>
</div>
</ion-content>
</ion-app>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { expect } from '@playwright/test';
import { test } from '@utils/test/playwright';

test.describe('item-sliding: scroll-target', () => {
test('should not scroll when the item-sliding is swiped in custom scroll target', async ({
page,
browserName,
}, testInfo) => {
test.skip(browserName === 'webkit', 'mouse.wheel is not available in WebKit');
test.skip(testInfo.project.metadata.rtl === true, 'This feature does not have RTL-specific behaviors');

await page.goto(`/src/components/item-sliding/test/scroll-target`);

const itemSlidingEl = page.locator('ion-item-sliding');
const scrollEl = page.locator('.ion-content-scroll-host');

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

const box = (await itemSlidingEl.boundingBox())!;
const centerX = box.x + box.width / 2;
const centerY = box.y + box.height / 2;

await page.mouse.move(centerX, centerY);
await page.mouse.down();
await page.mouse.move(centerX - 30, centerY);

/**
* Do not use scrollToBottom() or other scrolling methods
* on ion-content as those will update the scroll position.
* Setting scrollTop still works even with overflow-y: hidden.
* However, simulating a user gesture should not scroll the content.
*/
await page.mouse.wheel(0, 100);
await page.waitForChanges();

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