Skip to content
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
56 changes: 38 additions & 18 deletions core/src/components/action-sheet/action-sheet.tsx
Comment thread
brandyscarney marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
isCancel,
prepareOverlay,
present,
restoreRootFocusTrapAccessibility,
safeCall,
setOverlayId,
} from '@utils/overlays';
Expand Down Expand Up @@ -455,6 +456,15 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
connectedCallback() {
prepareOverlay(this.el);
this.triggerChanged();

// `componentDidLoad` only fires once per instance, so a reconnect has to
// rebuild the gesture its disconnect destroyed.
this.setupButtonActiveGesture();

// Re-apply the root lock if moved without dismiss() being called
if (this.presented) {
restoreRootFocusTrapAccessibility(this.el);
}
}

disconnectedCallback() {
Expand All @@ -478,26 +488,36 @@ export class ActionSheet implements ComponentInterface, OverlayInterface {
this.buttonsChanged();
}

componentDidLoad() {
/**
* Only create gesture if:
* 1. A gesture does not already exist
* 2. App is running in iOS mode
* 3. A wrapper ref exists
* 4. A group ref exists
*/
/**
* Only create gesture if:
* 1. A gesture does not already exist
* 2. App is running in iOS mode
* 3. A wrapper ref exists
* 4. A group ref exists
* 5. The host is still connected, since a reconnect can schedule this and
* disconnect again before the task runs
*/
private setupButtonActiveGesture() {
const { groupEl, wrapperEl } = this;
if (!this.gesture && getIonMode(this) === 'ios' && wrapperEl && groupEl) {
readTask(() => {
const isScrollable = groupEl.scrollHeight > groupEl.clientHeight;
if (!isScrollable) {
this.gesture = createButtonActiveGesture(wrapperEl, (refEl: HTMLElement) =>
refEl.classList.contains('action-sheet-button')
);
this.gesture.enable(true);
}
});
if (getIonMode(this) !== 'ios' || !wrapperEl || !groupEl) {
return;
}
readTask(() => {
// Bail if a call queued ahead of this one already built the gesture
// (a second would orphan the first with its listeners still bound), if
// the host disconnected while this task waited, or if the group scrolls.
if (this.gesture || !this.el.isConnected || groupEl.scrollHeight > groupEl.clientHeight) {
return;
}
this.gesture = createButtonActiveGesture(wrapperEl, (refEl: HTMLElement) =>
refEl.classList.contains('action-sheet-button')
);
this.gesture.enable(true);
});
}

componentDidLoad() {
this.setupButtonActiveGesture();

/**
* If action sheet was rendered with isOpen="true"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { expect } from '@playwright/test';
import { configs, test } from '@utils/test/playwright';
import { configs, detachAndReattach, test } from '@utils/test/playwright';

import { ActionSheetFixture } from './fixture';

Expand Down Expand Up @@ -166,3 +166,56 @@ configs({ directions: ['ltr'] }).forEach(({ title, screenshot, config }) => {
});
});
});

/**
* The button gesture only exists in iOS mode, so these run there.
*/
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ config, title }) => {
test.describe(title('action sheet: moved while presented'), () => {
test.beforeEach(async ({ page }) => {
await page.goto('/src/components/action-sheet/test/basic', config);
});

test('should keep the app root locked', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/31389',
});

const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent');

await page.click('#basic');
await ionActionSheetDidPresent.next();

await expect(page.locator('body')).toHaveClass(/backdrop-no-scroll/);

await detachAndReattach(page.locator('ion-action-sheet'));

await expect(page.locator('body')).toHaveClass(/backdrop-no-scroll/);
});

test('should keep activating buttons on press', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/31389',
});

const ionActionSheetDidPresent = await page.spyOnEvent('ionActionSheetDidPresent');

await page.click('#basic');
await ionActionSheetDidPresent.next();

await detachAndReattach(page.locator('ion-action-sheet'));

const button = page.locator('ion-action-sheet .action-sheet-button').first();
const box = (await button.boundingBox())!;

await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2);
await page.mouse.down();

await expect(button).toHaveClass(/ion-activated/);

await page.mouse.up();
});
});
});
28 changes: 19 additions & 9 deletions core/src/components/alert/alert.tsx
Comment thread
brandyscarney marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
isCancel,
prepareOverlay,
present,
restoreRootFocusTrapAccessibility,
safeCall,
setOverlayId,
} from '@utils/overlays';
Expand Down Expand Up @@ -363,10 +364,16 @@ export class Alert implements ComponentInterface, OverlayInterface {
this.triggerChanged();
/**
* If the alert was previously connected and is being reattached, the
* ResizeObserver was disconnected. componentDidLoad only fires once per
* instance, so re-establish the observer here on reconnect.
* `ResizeObserver` and the button gesture were torn down. `componentDidLoad`
* only fires once per instance, so re-establish both here on reconnect.
*/
this.setupButtonGroupResizeObserver();
this.setupButtonActiveGesture();

// Re-apply the root lock if moved without dismiss() being called
if (this.presented) {
restoreRootFocusTrapAccessibility(this.el);
}
}

componentWillLoad() {
Expand Down Expand Up @@ -394,20 +401,23 @@ export class Alert implements ComponentInterface, OverlayInterface {
this.buttonGroupResizeObserver = undefined;
}

componentDidLoad() {
/**
* Only create gesture if:
* 1. A gesture does not already exist
* 2. App is running in iOS mode
* 3. A wrapper ref exists
*/
/**
* Only create gesture if:
* 1. A gesture does not already exist
* 2. App is running in iOS mode
* 3. A wrapper ref exists
*/
private setupButtonActiveGesture() {
if (!this.gesture && getIonMode(this) === 'ios' && this.wrapperEl) {
this.gesture = createButtonActiveGesture(this.wrapperEl, (refEl: HTMLElement) =>
refEl.classList.contains('alert-button')
);
this.gesture.enable(true);
}
}

componentDidLoad() {
this.setupButtonActiveGesture();
this.setupButtonGroupResizeObserver();

/**
Expand Down
55 changes: 54 additions & 1 deletion core/src/components/alert/test/basic/alert.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { expect } from '@playwright/test';
import type { Locator } from '@playwright/test';
import type { E2EPage } from '@utils/test/playwright';
import { configs, test } from '@utils/test/playwright';
import { configs, detachAndReattach, test } from '@utils/test/playwright';

configs({ directions: ['ltr'] }).forEach(({ config, screenshot, title }) => {
test.describe(title('alert: basic'), () => {
Expand Down Expand Up @@ -203,3 +203,56 @@ class AlertFixture {
await expect(this.alert).toHaveScreenshot(screenshotFn(`alert-${modifier}`));
}
}

/**
* The button gesture only exists in iOS mode, so these run there.
*/
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ config, title }) => {
test.describe(title('alert: moved while presented'), () => {
test.beforeEach(async ({ page }) => {
await page.goto('/src/components/alert/test/basic', config);
});

test('should keep the app root locked', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/31389',
});

const ionAlertDidPresent = await page.spyOnEvent('ionAlertDidPresent');

await page.click('#basic');
await ionAlertDidPresent.next();

await expect(page.locator('body')).toHaveClass(/backdrop-no-scroll/);

await detachAndReattach(page.locator('ion-alert'));

await expect(page.locator('body')).toHaveClass(/backdrop-no-scroll/);
});

test('should keep activating buttons on press', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/31389',
});

const ionAlertDidPresent = await page.spyOnEvent('ionAlertDidPresent');

await page.click('#multipleButtons');
await ionAlertDidPresent.next();

await detachAndReattach(page.locator('ion-alert'));

const button = page.locator('ion-alert .alert-button').first();
const box = (await button.boundingBox())!;

await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2);
await page.mouse.down();

await expect(button).toHaveClass(/ion-activated/);

await page.mouse.up();
});
});
});
6 changes: 6 additions & 0 deletions core/src/components/loading/loading.tsx
Comment thread
brandyscarney marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
eventMethod,
prepareOverlay,
present,
restoreRootFocusTrapAccessibility,
setOverlayId,
} from '@utils/overlays';
import { sanitizeDOMString } from '@utils/sanitization';
Expand Down Expand Up @@ -208,6 +209,11 @@ export class Loading implements ComponentInterface, OverlayInterface {
connectedCallback() {
prepareOverlay(this.el);
this.triggerChanged();

// Re-apply the root lock if moved without dismiss() being called
if (this.presented) {
restoreRootFocusTrapAccessibility(this.el);
}
}

componentWillLoad() {
Expand Down
29 changes: 28 additions & 1 deletion core/src/components/loading/test/basic/loading.e2e.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expect } from '@playwright/test';
import type { E2EPage, ScreenshotFn } from '@utils/test/playwright';
import { configs, test } from '@utils/test/playwright';
import { configs, detachAndReattach, test } from '@utils/test/playwright';

const runVisualTest = async (page: E2EPage, selector: string, screenshot: ScreenshotFn, screenshotModifier: string) => {
const ionLoadingDidPresent = await page.spyOnEvent('ionLoadingDidPresent');
Expand Down Expand Up @@ -100,3 +100,30 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, screenshot, c
});
});
});

/**
* This behavior does not vary across modes/directions.
*/
configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ config, title }) => {
test.describe(title('loading: moved while presented'), () => {
test('should keep the app root locked', async ({ page }, testInfo) => {
testInfo.annotations.push({
type: 'issue',
description: 'https://github.com/ionic-team/ionic-framework/issues/31389',
});

await page.goto('/src/components/loading/test/basic', config);
const ionLoadingDidPresent = await page.spyOnEvent('ionLoadingDidPresent');

// This one carries no duration, so it can't auto-dismiss mid-move.
await page.click('#backdrop-loading');
await ionLoadingDidPresent.next();

await expect(page.locator('body')).toHaveClass(/backdrop-no-scroll/);

await detachAndReattach(page.locator('ion-loading'));

await expect(page.locator('body')).toHaveClass(/backdrop-no-scroll/);
});
});
});
Loading
Loading