feat(item-sliding): add automatic full expand animation of items by OS-pedrolourenco · Pull Request #31036 · ionic-team/ionic-framework

OS-pedrolourenco · 2026-03-25T14:01:54Z

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

vercel · 2026-03-25T14:02:07Z

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

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

…verOptions (#31022) Issue number: resolves #31012 ---------   ## What is the current behavior? `ModalOptions` and `PopoverOptions` in `@ionic/angular` are non-generic type aliases. Using `ModalOptions<typeof MyComponent>` causes a typescript error, even though the core `@ionic/core` types accept a generic parameter. ## What is the new behavior? `ModalOptions` and `PopoverOptions` now forward the generic type parameter from their core counterparts, allowing usage like` ModalOptions<typeof MyComponent>`. The default parameter preserves backward compatibility and existing code using ModalOptions without a generic should continue to work. ## Does this introduce a breaking change? - [ ] Yes - [X] No ## Other information This was the initial and intended behavior, but got broken in #30899 unintentionally. This PR fixes the issue and creates a test to ensure it doesn't happen again. Current dev build: ``` 8.8.2-dev.11773931429.15b2a51c ```

Issue number: N/A ---------   ## What is the current behavior?  While working on the migration for the spinner to Ionic Modular, I noticed that we had a [Transform test page](https://github.com/ionic-team/ionic-framework/blob/2b5b9137fc164c2f3305e493510a884c0afbfcf0/core/src/components/spinner/test/transform/index.html#L5) without any context of why it was there. I found out that it's meant for a [bug](#19247) that was reported in v4 and we did have a [test](https://github.com/ionic-team/ionic-framework/pull/24643/changes#diff-7b7ff84d3845fbde015775aa2da960310e80f79ec01b1f4a5957d751eddce7c9R1) for it at some point but it was removed at a later date because it was [discovered](#25259) that it wasn't doing anything. So we don't have any coverage of it if there's a regression. ## What is the new behavior?  - Added a test to prevent a regression ## Does this introduce a breaking change? - [ ] Yes - [x] No  ## Other information  How to test: 1. Be on your local 2. Navigate to `spinner.scss` 3. Update the following code snippet: ```diff - :host(.spinner-circular) svg { + :host(.spinner-circular) { animation: spinner-circular linear infinite; } ``` 4. By making this code change, we are introducing the [original issue](https://github.com/ionic-team/ionic-framework/pull/24643/changes#diff-fa8f6fb72eceb39e2482c0dbc083f69ecdabd411be541c21947f8e8e9bf9ee48L118). 5. Run the test 6. Notice that it fails 7. Undo the code change 8. Run the test 9. Verify that it passes

Issue number: resolves internal ---------   When the user swipes to a previous month that has adjacent days and selects one of the adjacent days from a previous month, the datetime selects the day but fails to scroll to the previous month. When a user clicks on a previous adjacent day after swiping the month, the calendar scrolls to the previous month. The same is applied to the next month. - [ ] Yes - [x] No  The previous behavior was only reproducible on iOS real devices.

…30983) Closes #30913 ## Description When `ion-input-otp` has the `readonly` prop set, typing is correctly blocked but users are still able to delete characters using the Backspace or Delete keys. When `disabled` or `readonly` are set, users are still able to paste into the component. ## Changes - Added guard for `disabled` and `readonly` inside `onInput` and `onPaste` handler - Prevented default behavior for `Backspace` and `Delete` when `readonly` is `true` - Return in `onKeydown` when `disabled` is `true` - Added e2e tests verifying the behavior ## How to Test 1. Add `readonly` to `ion-input-otp` 2. Attempt to type → no input allowed 3. Press Backspace/Delete → no characters removed Behavior now matches expected readonly semantics. --------- Co-authored-by: Brandy Smith <6577830+brandyscarney@users.noreply.github.com>

brandyscarney

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

brandyscarney · 2026-03-26T17:29:44Z

core/src/components/item-sliding/test/full-swipe/index.html

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

brandyscarney · 2026-03-26T17:33:06Z

core/src/components/item-sliding/test/full-swipe/index.html

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


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.

brandyscarney · 2026-03-26T17:43:35Z

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

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


Suggested change

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

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

brandyscarney · 2026-03-26T17:43:40Z

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

  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


Suggested change

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

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

brandyscarney · 2026-03-26T17:43:45Z

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


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


Suggested change

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

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

brandyscarney · 2026-03-26T18:02:07Z

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

+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);


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

ionic-framework/core/src/components/avatar/test/shape/avatar.e2e.ts

Lines 10 to 36 in abe3b3f

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`));

});

brandyscarney · 2026-03-26T18:33:27Z

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

+      await dragElementBy(item, page, -190);
+      await page.waitForTimeout(FULL_ANIMATION_MS);
+
+      expect(ionSwipe.length).toBeGreaterThan(0);


Suggested change

expect(ionSwipe.length).toBeGreaterThan(0);

expect(ionSwipe.length).toBe(1);

Can we check for exactly once?

brandyscarney · 2026-03-26T18:33:33Z

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

+      await dragElementBy(item, page, 190);
+      await page.waitForTimeout(FULL_ANIMATION_MS);
+
+      expect(ionSwipe.length).toBeGreaterThan(0);


Suggested change

expect(ionSwipe.length).toBeGreaterThan(0);

expect(ionSwipe.length).toBe(1);

Can we check for exactly once?

brandyscarney · 2026-03-26T18:52:55Z

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

+      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;


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

ShaneK

Good start! Noticed some accidental breaking changes that will need to be addressed first though

ShaneK · 2026-03-27T14:05:17Z

core/src/components/item-sliding/item-sliding.tsx

    }

-    const state = this.state;
    this.setOpenAmount(restingPoint, true);


The original onEnd fired fireSwipeEvent() for any full swipe that reached SwipeEnd/SwipeStart state, regardless of whether any option was expandable. Now that event emission is entirely inside animateFullSwipe, which only runs when hasExpandableOptions is true, non-expandable option sets will never emit ionSwipe. Apps using (ionSwipe) on a plain ion-item-options will silently stop receiving it. Shouldn't the fall-through path (when shouldTriggerFullSwipe is false) still fire the event when state has SwipeEnd/SwipeStart?

We should add some tests for this. I forgot we didn't have tests for events firing.

ShaneK · 2026-03-27T14:05:40Z

core/src/components/item-sliding/item-sliding.tsx

+      this.state = SlidingState.Disabled;
+      openSlidingItem = undefined;
+    } finally {
+      el.classList.remove('item-sliding-full-swipe');


openSlidingItem = undefined is inside the try block (line 348) so if anything throws before reaching it, the module-level reference stays pointing to this element. That prevents any other sliding item in the list from opening until something eventually clears it. Shouldn't this move into finally?

ShaneK · 2026-03-27T14:05:40Z

core/src/components/item-sliding/item-sliding.tsx

+      this.item.style.transition = `transform ${duration}ms ease-out`;
+      this.item.style.transform = `translate3d(${-position}px, 0, 0)`;
+
+      setTimeout(() => {


These setTimeout IDs aren't stored anywhere, so disconnectedCallback can't cancel them. If the component unmounts mid-animation, options.fireSwipeEvent() still fires when the timer resolves since options is captured by the closure before this.leftOptions/this.rightOptions gets cleared. There's already a this.tmr pattern for exactly this. Could the timer IDs get the same treatment?

ShaneK · 2026-03-27T14:05:41Z

core/src/components/item-sliding/item-sliding.tsx

+    }
+
+    const { el } = this;
+    el.classList.add('item-sliding-full-swipe');


Is there a CSS rule for item-sliding-full-swipe somewhere? I don't see one in item-sliding.scss or the platform-specific variants. If it's needed so the item can animate past the list's clipping boundary (e.g. overflow: visible on the host), the rule is missing. If it's a hook for external consumers, worth a comment. Otherwise it could be dropped.

ShaneK · 2026-03-27T14:06:03Z

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

+      await dragElementBy(item, page, -190);
+      await page.waitForTimeout(FULL_ANIMATION_MS);
+
+      expect(ionSwipe.length).toBeGreaterThan(0);


The codebase has toHaveReceivedEvent(), toHaveReceivedEventTimes(1), and not.toHaveReceivedEvent() in core/src/utils/test/playwright/matchers/. Could you use those instead of .length checks? The non-expandable test on line 62 would become expect(ionSwipe).not.toHaveReceivedEvent() as well.

ShaneK · 2026-03-27T14:06:04Z

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

+ */
+
+// Full animation cycle duration (100ms expand + 250ms off-screen + 300ms delay + 250ms return)
+const FULL_ANIMATION_MS = 1100;


FULL_ANIMATION_MS = 1100 is the sum of the hardcoded delays in animateFullSwipe (100 + 250 + 300 + 250). If any of those change, these tests start silently flaking. For the tests that check ionSwipe, could you use await ionSwipe.next() instead? That resolves as soon as the event fires without sleeping for the full cycle. The closed-state test could then follow up with page.waitForChanges().

OS-pedrolourenco added 4 commits March 23, 2026 18:39

First checkpoint

6017570

Second checkpoint

994b969

Add automatic full expand animation to items

a6f271f

Tweaks

63274c0

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

OS-pedrolourenco added 2 commits March 25, 2026 16:39

Fix disabled option bug + add tests

bd8568f

Fix lint issues

87ddc89

vercel bot deployed to Preview March 25, 2026 17:07 View deployment

Merge branch 'refs/heads/next' into ROU-12664

e0736dc

vercel bot deployed to Preview March 26, 2026 10:08 View deployment

Skip flaky tests

258e9df

vercel bot deployed to Preview March 26, 2026 15:46 View deployment

OS-pedrolourenco requested a review from brandyscarney March 26, 2026 16:04

OS-pedrolourenco marked this pull request as ready for review March 26, 2026 16:04

OS-pedrolourenco requested a review from a team as a code owner March 26, 2026 16:04

Merge branch 'next' into ROU-12664

0e9abb2

vercel bot deployed to Preview March 26, 2026 16:08 View deployment

ShaneK and others added 11 commits March 26, 2026 16:13

v8.8.2

9aaecc9

chore(): update package lock files

24861e5

chore(): add updated snapshots

f76e46e

chore(): add updated snapshots

4cb32a1

chore: sync next with main (#31040)

51db026

Merge branch 'next' into ROU-12664

7762854

vercel bot deployed to Preview March 26, 2026 17:17 View deployment

brandyscarney requested changes Mar 26, 2026

View reviewed changes

ShaneK requested changes Mar 27, 2026

View reviewed changes

brandyscarney force-pushed the next branch from 51db026 to 06ace54 Compare March 27, 2026 15:26

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

	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`));
	});

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

Conversation

OS-pedrolourenco commented Mar 25, 2026 • edited by brandyscarney Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is the current behavior?

What is the new behavior?

Does this introduce a breaking change?

Other information

Uh oh!

vercel bot commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

brandyscarney left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaneK left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

OS-pedrolourenco commented Mar 25, 2026 •

edited by brandyscarney

Loading

vercel bot commented Mar 25, 2026 •

edited

Loading