Skip to content

Commit

Permalink
fix(item-sliding): swiping inside of virtual scroller now prevents sc…
Browse files Browse the repository at this point in the history
…rolling (#25345)
  • Loading branch information
liamdebeasi authored May 31, 2022
1 parent d5cde5e commit 5a1a5f6
Show file tree
Hide file tree
Showing 5 changed files with 175 additions and 25 deletions.
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);
});
});

0 comments on commit 5a1a5f6

Please sign in to comment.