Skip to content

feat(item-sliding): add automatic full expand animation of items#31036

Open
OS-pedrolourenco wants to merge 10 commits intonextfrom
ROU-12664
Open

feat(item-sliding): add automatic full expand animation of items#31036
OS-pedrolourenco wants to merge 10 commits intonextfrom
ROU-12664

Conversation

@OS-pedrolourenco
Copy link
Contributor

@OS-pedrolourenco OS-pedrolourenco commented Mar 25, 2026

Issue number: internal


What is the current behavior?

Dragging an ion-item with an expandable option would cause a full swipe event to be triggered but also cause the item to return to a position just after the item options once the mouse button was released.

What is the new behavior?

  • Now, once the new threshold is passed and the mouse button is released, a full expand animation will take place and, after it is finished, only then is the full swipe event triggered and the item returns to its initial state where no options are visible.

Does this introduce a breaking change?

  • Yes
  • No

Other information

Preview

@github-actions github-actions bot added the package: core @ionic/core package label Mar 25, 2026
@vercel
Copy link

vercel bot commented Mar 25, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
ionic-framework Ready Ready Preview, Comment Mar 26, 2026 5:17pm

Request Review

@OS-pedrolourenco OS-pedrolourenco marked this pull request as ready for review March 26, 2026 16:04
@OS-pedrolourenco OS-pedrolourenco requested a review from a team as a code owner March 26, 2026 16:04
Copy link
Member

@brandyscarney brandyscarney left a comment

Choose a reason for hiding this comment

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

The functionality is working well. 🎉 Requested some changes related to the tests!

Copy link
Member

Choose a reason for hiding this comment

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

There is some weirdness going on with the headings in ios:

Image


<ion-content>
<div class="ion-padding-start" style="padding-top: 30px">
<ion-title>Full Swipe - Expandable Options</ion-title>
Copy link
Member

Choose a reason for hiding this comment

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

You shouldn't use ion-title outside of a toolbar, this is what's causing ios to look strange. Either use an h2 like this:

<h2>Full Swipe - Expandable Options</h2>

<style>
  h2 {
    font-size: 12px;
    font-weight: normal;

    color: #6f7378;

    margin-top: 10px;
    margin-left: 5px;
  }
</style>

Or use an ion-list-header at the top of the list.

});
test.describe('start options', () => {
test('should not have visual regressions', async ({ page }) => {
//TODO(FW-7184): remove skip once issue is resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//TODO(FW-7184): remove skip once issue is resolved
// TODO(FW-7184): remove skip once issue is resolved

test.describe(title('item-sliding: basic'), () => {
test.describe('safe area left', () => {
test('should have padding on the left only', async ({ page }) => {
//TODO(FW-7184): remove skip once issue is resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//TODO(FW-7184): remove skip once issue is resolved
// TODO(FW-7184): remove skip once issue is resolved


test.describe('safe area right', () => {
test('should have padding on the right only', async ({ page }) => {
//TODO(FW-7184): remove skip once issue is resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
//TODO(FW-7184): remove skip once issue is resolved
// TODO(FW-7184): remove skip once issue is resolved

configs({ modes: ['ios', 'md', 'ionic-md'], directions: ['ltr'] }).forEach(({ title, config }) => {
test.describe(title('item-sliding: full swipe'), () => {
test.beforeEach(async ({ page }) => {
await page.goto(`/src/components/item-sliding/test/full-swipe`, config);
Copy link
Member

Choose a reason for hiding this comment

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

Can we break this into individual examples using page.setContent? Like this:

test('should not have visual regressions when containing text', async ({ page }) => {
await page.setContent(
`
<ion-avatar shape="round">AB</ion-avatar>
`,
config
);
const avatar = page.locator('ion-avatar');
await expect(avatar).toHaveScreenshot(screenshot(`avatar-shape-round-text`));
});
test('should not have visual regressions when containing an icon', async ({ page }) => {
await page.setContent(
`
<ion-avatar shape="round">
<ion-icon name="person-outline"></ion-icon>
</ion-avatar>
`,
config
);
const avatar = page.locator('ion-avatar');
await expect(avatar).toHaveScreenshot(screenshot(`avatar-shape-round-icon`));
});

await dragElementBy(item, page, -190);
await page.waitForTimeout(FULL_ANIMATION_MS);

expect(ionSwipe.length).toBeGreaterThan(0);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expect(ionSwipe.length).toBeGreaterThan(0);
expect(ionSwipe.length).toBe(1);

Can we check for exactly once?

await dragElementBy(item, page, 190);
await page.waitForTimeout(FULL_ANIMATION_MS);

expect(ionSwipe.length).toBeGreaterThan(0);
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
expect(ionSwipe.length).toBeGreaterThan(0);
expect(ionSwipe.length).toBe(1);

Can we check for exactly once?

const item = page.locator('#expandable-end');

// In RTL the "end" side is on the left, revealed by dragging right
const dragByX = config.direction === 'rtl' ? 190 : -190;
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need a separate test for RTL? We should be able to use this on all the other tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants