-
Notifications
You must be signed in to change notification settings - Fork 13.3k
feat(modal): expose isDismissing on ionDragEnd #31002
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
base: major-9.0
Are you sure you want to change the base?
Changes from all commits
c200d78
4d87d5e
8d7013b
ae4023f
ed90f22
98fd358
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,4 +72,13 @@ export interface ModalDragEventDetail { | |
| * the gesture. | ||
| */ | ||
| snapBreakpoint?: number; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| /** | ||
| * Whether the modal is attempting to dismiss when the drag gesture | ||
| * ends. A `canDismiss` callback can still cancel the dismiss after | ||
| * this event is emitted, so use `ionModalDidDismiss` to confirm that | ||
| * the modal actually closed. | ||
| * | ||
| * This is only set on `ionDragEnd`. | ||
| */ | ||
| isDismissing?: boolean; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sheet modals never set this, so a consumer writing the obvious Both gestures publish through the same isDismissing: snapBreakpoint === 0 && !canDismissBlocksGesture,Same two values Card
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -107,13 +107,15 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, c | |
| }); | ||
|
|
||
| test.describe(title('card modal: drag events'), () => { | ||
| test('should emit ionDragStart, ionDragMove, and ionDragEnd events', async ({ page }) => { | ||
| await page.goto('/src/components/modal/test/card', config); | ||
| let cardModalPage: CardModalPage; | ||
|
|
||
| const ionModalDidPresent = await page.spyOnEvent('ionModalDidPresent'); | ||
| test.beforeEach(async ({ page }) => { | ||
| cardModalPage = new CardModalPage(page); | ||
| await cardModalPage.navigate('/src/components/modal/test/card', config); | ||
| }); | ||
|
|
||
| await page.click('#drag-events'); | ||
| await ionModalDidPresent.next(); | ||
| test('should emit ionDragStart, ionDragMove, and ionDragEnd events', async ({ page }) => { | ||
| await cardModalPage.openModalByTrigger('#drag-events'); | ||
|
|
||
| const ionDragStart = await page.spyOnEvent('ionDragStart'); | ||
| const ionDragMove = await page.spyOnEvent('ionDragMove'); | ||
|
|
@@ -130,7 +132,7 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, c | |
| expect(ionDragStart.length).toBe(1); | ||
|
|
||
| expect(ionDragMove.length).toBeGreaterThan(0); | ||
| expect(Object.keys(dragMoveEvent.detail).length).toBe(4); | ||
| expect(Object.keys(dragMoveEvent.detail).sort()).toEqual(['currentY', 'deltaY', 'progress', 'velocityY']); | ||
|
|
||
| expect(ionDragEnd.length).toBe(0); | ||
|
|
||
|
|
@@ -148,7 +150,44 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, c | |
| expect(ionDragMove.length).toBeGreaterThan(0); | ||
|
|
||
| expect(ionDragEnd.length).toBe(1); | ||
| expect(Object.keys(dragEndEvent.detail).length).toBe(4); | ||
| expect(Object.keys(dragEndEvent.detail).sort()).toEqual([ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This only checks the key set, so it passes whether the implementation emits The fixture already handles both outcomes, and the suite already proves which distances land either side of the threshold: test('should report isDismissing true when the gesture dismisses the modal', async ({ page }) => {
await dragElementBy(page.locator('.modal-card ion-header'), page, 0, 300);
const ev = await ionDragEnd.next();
expect(ev.detail.isDismissing).toBe(true);
});
test('should report isDismissing false when the modal settles back open', async ({ page }) => {
await dragElementBy(page.locator('.modal-card ion-header'), page, 0, 20);
const ev = await ionDragEnd.next();
expect(ev.detail.isDismissing).toBe(false);
await expect(page.locator('ion-modal')).toBeVisible();
});I ran both against a local build with this patch applied and they pass. Worth pairing each with the visibility check so the test proves the field matches reality instead of matching itself. A third one for the Swapping
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
| 'currentY', | ||
| 'deltaY', | ||
| 'isDismissing', | ||
| 'progress', | ||
| 'velocityY', | ||
| ]); | ||
| }); | ||
|
|
||
| test('should report isDismissing as true when the gesture dismisses the modal', async ({ page }) => { | ||
| await cardModalPage.openModalByTrigger('#card'); | ||
|
|
||
| const ionDragEnd = await page.spyOnEvent('ionDragEnd'); | ||
|
|
||
| /** | ||
| * `swipeToCloseModal` drags 300px and waits for `ionModalDidDismiss`, | ||
| * so the modal is known to have dismissed by the time the event | ||
| * detail is checked. | ||
| */ | ||
| await cardModalPage.swipeToCloseModal('ion-modal ion-header'); | ||
|
|
||
| const dragEndEvent = await ionDragEnd.next(); | ||
|
|
||
| expect(dragEndEvent.detail.isDismissing).toBe(true); | ||
| }); | ||
|
|
||
| test('should report isDismissing as false when the modal settles back open', async ({ page }) => { | ||
| const modal = await cardModalPage.openModalByTrigger('#card'); | ||
|
|
||
| const ionDragEnd = await page.spyOnEvent('ionDragEnd'); | ||
|
|
||
| // 20px is short enough that the gesture never passes the dismiss threshold | ||
| await cardModalPage.swipeToCloseModal('ion-modal ion-header', false, 20); | ||
|
|
||
| const dragEndEvent = await ionDragEnd.next(); | ||
|
|
||
| expect(dragEndEvent.detail.isDismissing).toBe(false); | ||
| await expect(modal).toBeVisible(); | ||
| }); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding this variable changes the
ionDragEnddetail length, so the existing assertion inmodal-card.e2e.tswill fail. Please update the expected length there.